Skip to content

Fix nvpair-tui panic when a table view seeds rows before its first resize - #20

Open
habakan wants to merge 1 commit into
NVIDIA:developfrom
habakan:fix/nvpair-tui-errors-view-nil-columns-panic
Open

habakan wants to merge 1 commit into
NVIDIA:developfrom
habakan:fix/nvpair-tui-errors-view-nil-columns-panic

Conversation

@habakan

@habakan habakan commented Sep 5, 2026

Copy link
Copy Markdown

This is my first contribution to this repository, so please redirect me if this isn't the right approach.

Outcome

Fixes a reliable startup crash in nvpair-tui. Every table-backed view (Errors, Nodes, Cluster, Manual, Workloads, Engines, Proxies) builds its table.Model via newTable(nil) and only sets real columns inside SetSize, which the Bubble Tea runtime calls from the first tea.WindowSizeMsg. Each view's Init() fires an RPC call (errors:get-initial, nodes:get-initial, etc.) at the same time, and nothing orders that response against the first WindowSizeMsg. When the RPC reply arrives first — which happens reliably whenever the broker already holds at least one stored item at startup (e.g. one prior service error) — the view's set*() handler calls table.SetRows on a still-zero-column table, and bubbles/table.renderRow panics with index out of range [0] with length 0, killing the whole TUI.

Fix: seed each table with real columns at construction time instead of nil, so SetRows never sees zero columns regardless of message ordering.

  • ui/errors.go: build via v.columns() instead of nil (it already computes columns from v.width, clamped for a zero width).
  • ui/nodes.go, ui/cluster.go, ui/manualnodes.go, ui/workloads.go, ui/engines.go, ui/proxies.go: call v.SetSize(0, 0) right after construction, running the same SetColumns each view's own SetSize already performs on a real resize.

Bumped nvpair-tui 0.7.2 -> 0.7.3 (PATCH: bug fix, no IPC/HTTP change) and product/installer 0.91.7 -> 0.91.8 accordingly.

In scope / out of scope

In scope: the seven newTable(nil) sites in nvpair-tui/ui. Out of scope: any broader change to how views are sized or how the broker reports errors.

Validation environment

Linux x86_64, Go 1.25.0.

cd services/nvpair-tui && go build ./... && go vet ./... && go test ./...   # all pass
node scripts/spdx-headers.mjs                                              # 871 checked, 0 missing
cd services/tests && go test ./...

The cross-process suite has 7 pre-existing failures (TestBrokerRestoresClusterIdentityAfterRestart, TestLogSetLevelViaRPC, TestLogLevelEnvFallback, TestWorkloadManagerInboundRelay, TestWorkloadOutOfOrderSuppressed, TestWorkloadFailedOnNodeLoss, TestWorkloadManagerOutboundBroadcast). I confirmed these reproduce identically on main without this change, so they're unrelated to this fix.

Manually reproduced the panic against the published 0.1.1 .deb (a real broker with a stored service error present crashes the stock nvpair-tui on every launch), then confirmed a nvpair-tui built with this patch starts cleanly against the same broker and on-disk state.

Compatibility and security risk

None expected — this only changes when a table's columns are initialized, not any wire format, IPC contract, or stored data.

Documentation

No user-facing behavior changes beyond "it no longer crashes," so no docs changes.

…size

Every table-backed view built its table.Model via newTable(nil) and only
set real columns inside SetSize (called on the first tea.WindowSizeMsg).
Each view's Init() fires an RPC call at the same time, and if that reply
arrives first — which happens reliably whenever the broker already holds
one stored item at startup — SetRows runs on a zero-column table and
bubbles/table.renderRow panics, killing the TUI.

Seed real columns at construction time instead of nil, so SetRows never
sees zero columns regardless of ordering.

Bump nvpair-tui 0.7.2 -> 0.7.3 (PATCH) and product/installer 0.91.7 ->
0.91.8 accordingly.

Signed-off-by: habakan <kansukebano@gmail.com>
@Noah-Tervalon-Nvidia

Copy link
Copy Markdown
Collaborator

Hi, thank you so much for raising this. I've been working on a MASSIVE TUI overhaul over the last few weeks and actually already resolved this issue. I would love if when I bring that into the github here you can double check this for me to make sure it stays resolved!

@Noah-Tervalon-Nvidia
Noah-Tervalon-Nvidia changed the base branch from main to develop September 21, 2026 21:54
@Noah-Tervalon-Nvidia

Copy link
Copy Markdown
Collaborator

@habakan can you check out the update here: #117 - it's really big, but it has this fix kind of by default since it changes some of how things are displayed. Would love to get more people checking it out just to really pound it before we merge it in.

@habakan

habakan commented Sep 24, 2026 •

Copy link
Copy Markdown
Author

Thanks for folding this into the overhaul! I gave #117 (a6bc294) a try.

On main, feeding the Errors view a row before its first resize still panics with index out of range [0] with length 0, and on #117 it doesn't. I also saw that TestViewsAcceptRowsBeforeResize in ui/table_test.go already covers this for every table view, so nothing extra needed there. I built it in a Linux container and ran the TUI with an error present at startup. It came up fine with Errors (1) in the tab bar, and go test ./... passes.

Happy to close this one, or now if you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants