diff --git a/internal/connections/client.go b/internal/connections/client.go index 63617868..c72caa87 100644 --- a/internal/connections/client.go +++ b/internal/connections/client.go @@ -192,21 +192,33 @@ func (s *Client) List(ctx context.Context, sort SortMode) (ConnectionList, error } deadline := time.Now().Add(s.retryBudget) + var partial ConnectionList + havePartial := false for { list, retryAfter, err := s.tryList(ctx, sort) - if err == nil { + switch { + case err == nil && !list.HasUnreachableInstance(): return list, nil - } - if isTimeoutError(err) && ctx.Err() != nil { + case err == nil: + // 200 OK, but the server flagged one or more instances unreachable + // (a partial fan-out failure). These are usually momentary under + // load and clear within a poll or two, so retry within the budget. + // Retry locally; the response carries no Retry-After hint. + partial, havePartial = list, true + retryAfter = 0 + case isTimeoutError(err) && ctx.Err() != nil: if callerCtx.Err() != nil { return ConnectionList{}, fmt.Errorf("list connections: %w", callerCtx.Err()) } return ConnectionList{}, fmt.Errorf("list connections: request timed out after %s, please retry", s.cfg.RequestTimeout) - } - if !errors.Is(err, errListWarming) { + case !errors.Is(err, errListWarming): return ConnectionList{}, err } + if s.retryBudget <= 0 { + if havePartial { + return partial, nil + } return ConnectionList{}, errListWarmingExhausted } @@ -215,12 +227,21 @@ func (s *Client) List(ctx context.Context, sort SortMode) (ConnectionList, error delay = s.retryDelay() } if time.Now().Add(delay).After(deadline) { + if havePartial { + return partial, nil + } return ConnectionList{}, errListWarmingExhausted } select { case <-time.After(delay): case <-ctx.Done(): + if callerCtx.Err() != nil { + return ConnectionList{}, fmt.Errorf("list connections: %w", callerCtx.Err()) + } + if havePartial { + return partial, nil + } return ConnectionList{}, fmt.Errorf("list connections: %w", ctx.Err()) } } diff --git a/internal/connections/client_test.go b/internal/connections/client_test.go index c2b273aa..9321a9ca 100644 --- a/internal/connections/client_test.go +++ b/internal/connections/client_test.go @@ -20,7 +20,7 @@ const sampleListResponse = `{ "captured_at": "2026-04-29T12:34:56.789Z", "instances": [ {"id": "primary", "role": "primary", "error": null}, - {"id": "replica-1", "role": "replica", "error": "timeout after 2s"} + {"id": "replica-1", "role": "replica", "error": null} ], "data": [ { @@ -180,8 +180,8 @@ func TestClient_ListDecodesConnectionList(t *testing.T) { if list.Instances[0] != (InstanceMeta{ID: "primary", Role: "primary"}) { t.Errorf("Instances[0] = %+v, want primary/primary/(no error)", list.Instances[0]) } - if list.Instances[1] != (InstanceMeta{ID: "replica-1", Role: "replica", Error: "timeout after 2s"}) { - t.Errorf("Instances[1] = %+v, want replica-1/replica/timeout", list.Instances[1]) + if list.Instances[1] != (InstanceMeta{ID: "replica-1", Role: "replica"}) { + t.Errorf("Instances[1] = %+v, want replica-1/replica/(no error)", list.Instances[1]) } if first.InstanceRole != "primary" { @@ -526,6 +526,103 @@ func TestClientListPartialResponseFriendlyMessage(t *testing.T) { } } +// partialInstanceListResponse models a 200 OK that the server returns while a +// fan-out to one instance failed: the primary is flagged unreachable while the +// replica reports cleanly. The error string mirrors a real captured trace. +const partialInstanceListResponse = `{ + "type": "list", + "captured_at": "2026-04-29T12:34:56.789Z", + "instances": [ + {"id": "primary", "role": "primary", "error": "remote service unavailable"}, + {"id": "replica-1", "role": "replica", "error": null} + ], + "data": [] +}` + +func TestClient_ListRetriesPartialInstanceErrorThenReturnsClean(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + if calls.Add(1) == 1 { + _, _ = io.WriteString(w, partialInstanceListResponse) + return + } + _, _ = io.WriteString(w, sampleListResponse) + })) + defer srv.Close() + + c := newClientWithTimings(t, srv.URL, 500*time.Millisecond, 10*time.Millisecond, 0) + list, err := c.List(context.Background(), SortByTransactionStart) + if err != nil { + t.Fatalf("List: %v", err) + } + if got := calls.Load(); got != 2 { + t.Fatalf("calls = %d, want 2 (retried past the partial-instance failure)", got) + } + for _, inst := range list.Instances { + if inst.Error != "" { + t.Fatalf("returned list still reports unreachable instance %q (%q), want a clean retry result", inst.ID, inst.Error) + } + } +} + +func TestClient_ListReturnsPartialAfterRetryBudgetExhausted(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, partialInstanceListResponse) + })) + defer srv.Close() + + c := newClientWithTimings(t, srv.URL, 40*time.Millisecond, 10*time.Millisecond, 0) + start := time.Now() + list, err := c.List(context.Background(), SortByTransactionStart) + elapsed := time.Since(start) + if err != nil { + t.Fatalf("List: want partial list, got error %v", err) + } + if calls.Load() < 2 { + t.Fatalf("calls = %d, want at least 2 (retried before giving up)", calls.Load()) + } + if elapsed > time.Second { + t.Fatalf("List took %v, want retry budget to expire promptly", elapsed) + } + // A persistent partial failure still returns useful data: the reachable + // instances plus the per-instance error that drives the unreachable banner. + var found bool + for _, inst := range list.Instances { + if inst.ID == "primary" && inst.Error == "remote service unavailable" { + found = true + } + } + if !found { + t.Fatalf("partial list lost the instance error; instances = %+v", list.Instances) + } +} + +func TestClient_ListDoesNotRetryPartialWhenBudgetDisabled(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, partialInstanceListResponse) + })) + defer srv.Close() + + c := newClientWithTimings(t, srv.URL, 0, 10*time.Millisecond, 0) + list, err := c.List(context.Background(), SortByTransactionStart) + if err != nil { + t.Fatalf("List: %v", err) + } + if got := calls.Load(); got != 1 { + t.Fatalf("calls = %d, want 1 (no retry when budget disabled)", got) + } + if got := list.Instances[0].Error; got != "remote service unavailable" { + t.Fatalf("Instances[0].Error = %q, want decoded instance error", got) + } +} + func TestClient_ListRetryWaitHonorsContextCancellation(t *testing.T) { var calls atomic.Int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -555,6 +652,36 @@ func TestClient_ListRetryWaitHonorsContextCancellation(t *testing.T) { } } +func TestClient_ListPartialRetryWaitHonorsContextCancellation(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, partialInstanceListResponse) + })) + defer srv.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + c := newClientWithTimings(t, srv.URL, 500*time.Millisecond, 250*time.Millisecond, 0) + start := time.Now() + _, err := c.List(ctx, SortByTransactionStart) + elapsed := time.Since(start) + if err == nil { + t.Fatal("List: want context error, got nil") + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want errors.Is(err, context.DeadlineExceeded)", err) + } + if got := calls.Load(); got != 1 { + t.Fatalf("calls = %d, want 1 because context expired during partial retry wait", got) + } + if elapsed > time.Second { + t.Fatalf("List took %v, want context cancellation to interrupt partial retry wait", elapsed) + } +} + func TestClient_ListHonorsRetryAfterHeader(t *testing.T) { var calls atomic.Int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { diff --git a/internal/connections/connection_list.go b/internal/connections/connection_list.go index 45ba3f78..dd9eddc8 100644 --- a/internal/connections/connection_list.go +++ b/internal/connections/connection_list.go @@ -43,6 +43,18 @@ type ConnectionList struct { Topology *Topology `json:"topology,omitempty"` } +// HasUnreachableInstance reports whether the server flagged any instance as +// unreachable in the list response. These partial fan-out failures drive the +// "N of M instances unreachable" banner. +func (l ConnectionList) HasUnreachableInstance() bool { + for _, inst := range l.Instances { + if inst.Error != "" { + return true + } + } + return false +} + type Connection struct { PID int `json:"pid"` Instance string `json:"instance"` diff --git a/internal/connections/tui/detail.go b/internal/connections/tui/detail.go index 49518b54..c7eebdd2 100644 --- a/internal/connections/tui/detail.go +++ b/internal/connections/tui/detail.go @@ -434,7 +434,9 @@ func blockerTreeLabel(row blockerRow, selected bool) string { // consistent across views. return selectedRowStyle.Render("▶ " + label) } - return label + // Reserve the caret's width on unselected rows so moving the selection + // doesn't shift the row text, matching the table view's fixed-width marker. + return " " + label } func wrapLines(text string, width int) []string { diff --git a/internal/connections/tui/detail_test.go b/internal/connections/tui/detail_test.go index 643fb87e..a22a53c0 100644 --- a/internal/connections/tui/detail_test.go +++ b/internal/connections/tui/detail_test.go @@ -688,3 +688,20 @@ func TestDetailHelpHidesRefreshWhilePaused(t *testing.T) { c.Assert(footer, qt.Not(qt.Contains), "r refresh") c.Assert(footer, qt.Contains, "space resume") } + +func TestBlockerTreeLabelReservesCaretWidth(t *testing.T) { + c := qt.New(t) + row := blockerRow{PID: 123, Present: false} + + // The selection caret ("▶ ") must not shift the row text: an unselected + // row reserves the caret's width so the label sits at the same column + // whether or not it is selected (matching the table view's fixed marker). + unselected := blockerTreeLabel(row, false) + c.Assert(unselected, qt.Equals, " "+blockerLabel(row)) + + // Selected rows use the caret + a space — the same two display columns as + // the unselected padding, so the label text never shifts between states. + selectedInner := ansi.Strip(blockerTreeLabel(row, true)) + c.Assert(strings.HasPrefix(selectedInner, "▶ "), qt.IsTrue) + c.Assert(strings.TrimPrefix(selectedInner, "▶ "), qt.Equals, strings.TrimPrefix(unselected, " ")) +} diff --git a/internal/connections/tui/model.go b/internal/connections/tui/model.go index 1a3169ae..b37919be 100644 --- a/internal/connections/tui/model.go +++ b/internal/connections/tui/model.go @@ -343,6 +343,15 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.consecutiveErrors++ return m, nil } + // A degraded result reached us after the client exhausted its retries on + // a partial-instance failure. Don't swap it into the live view: hold the + // last good frame and let the staleness dot and captured-age surface it, + // rather than flashing the "N unreachable" banner over good data. First + // load has no good frame to hold, so fall through and show what we have. + if m.hasList && msg.list.HasUnreachableInstance() { + m.consecutiveErrors++ + return m, nil + } m.consecutiveErrors = 0 m.initialAccessDenied = false cursor := m.samples.Push(msg.list) diff --git a/internal/connections/tui/model_test.go b/internal/connections/tui/model_test.go index 5e4305d4..104096ae 100644 --- a/internal/connections/tui/model_test.go +++ b/internal/connections/tui/model_test.go @@ -140,6 +140,46 @@ func TestModelKeepsLastListWhenRefreshFails(t *testing.T) { c.Assert(view, qt.Contains, "error: connection refused") } +func degradedList(captured time.Time, pid int) live.ConnectionList { + list := live.NewConnectionList(captured, []live.Connection{{PID: pid, Instance: "primary"}}, live.SortByTransactionStart) + list.Instances = []live.InstanceMeta{ + {ID: "primary", Role: "primary", Error: "remote service unavailable"}, + {ID: "replica-1", Role: "replica"}, + } + return list +} + +func TestModelHoldsLastGoodListOnPersistentPartial(t *testing.T) { + c := qt.New(t) + model := NewModel(context.Background(), &clientStub{}, time.Second, 0) + good := live.NewConnectionList(time.Now(), []live.Connection{{PID: 10, Instance: "primary"}}, live.SortByTransactionStart) + + updated, _ := model.Update(listMsg{list: good}) + // A degraded refresh (client retries already exhausted) must not replace the + // good frame or show the unreachable banner — it holds and goes stale. + updated, _ = updated.(Model).Update(listMsg{list: degradedList(time.Now(), 20)}) + got := updated.(Model) + view := got.View() + + c.Assert(view, qt.Contains, "10") + c.Assert(view, qt.Not(qt.Contains), "20") + c.Assert(view, qt.Not(qt.Contains), "unreachable") + c.Assert(got.consecutiveErrors, qt.Equals, 1) +} + +func TestModelShowsPartialOnFirstLoadWithNoPriorFrame(t *testing.T) { + c := qt.New(t) + model := NewModel(context.Background(), &clientStub{}, time.Second, 0) + + // No good frame to hold yet, so the first result is shown even if degraded. + updated, _ := model.Update(listMsg{list: degradedList(time.Now(), 30)}) + got := updated.(Model) + view := got.View() + + c.Assert(view, qt.Contains, "30") + c.Assert(view, qt.Contains, "unreachable") +} + func TestModelShowsInitialListErrorBeforeAnySuccessfulList(t *testing.T) { c := qt.New(t) model := NewModel(context.Background(), &clientStub{}, time.Second, 0) diff --git a/internal/connections/tui/table.go b/internal/connections/tui/table.go index d30b1c4f..1891caa6 100644 --- a/internal/connections/tui/table.go +++ b/internal/connections/tui/table.go @@ -8,7 +8,6 @@ import ( "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/lipgloss" - lgtable "github.com/charmbracelet/lipgloss/table" "github.com/charmbracelet/x/ansi" live "github.com/planetscale/cli/internal/connections" ) @@ -734,57 +733,36 @@ func renderConnectionTable(state tableState, bodyAvail int) string { selectedInSlice := state.Selected - start counts := live.BlockingCounts(connections) headers, rows := buildConnectionRowsForDisplay(state.DisplayPreset, visible, counts, width, selectedInSlice) - if state.DisplayPreset == connectionDisplayProcesslist { - return renderProcesslistConnectionTable(headers, rows, visible, selectedInSlice, width) - } - - return lgtable.New(). - Border(lipgloss.HiddenBorder()). - BorderTop(false). - BorderBottom(false). - BorderLeft(false). - BorderRight(false). - BorderHeader(false). - BorderColumn(false). - Headers(headers...). - Rows(rows...). - Width(width). - Wrap(false). - StyleFunc(func(row, col int) lipgloss.Style { - if row == lgtable.HeaderRow { - return tableCellStyle(headerStyle, headers, col) - } - if row >= 0 && row < len(visible) { - conn := visible[row] - style := connectionRowStyle(conn, counts[conn.PID], row == selectedInSlice) - style = connectionColumnStyleForDisplay(state.DisplayPreset, style, conn, counts[conn.PID], col, headers) - return tableCellStyle(style, headers, col) - } - return lipgloss.NewStyle() - }). - Render() + return renderConnectionTableTight(state.DisplayPreset, headers, rows, visible, counts, selectedInSlice, width) } -func renderProcesslistConnectionTable(headers []string, rows [][]string, connections []live.Connection, selectedInSlice int, width int) string { +// renderConnectionTableTight lays the table out with content-based column +// widths and a fixed inter-column gap, then clips to the terminal width. Unlike +// a width-filling table layout, this keeps columns packed at the left when the +// content (notably the trailing QUERY column) is short, so uniform/short rows +// don't get spread across the whole terminal. +func renderConnectionTableTight(display connectionDisplayPreset, headers []string, rows [][]string, connections []live.Connection, counts map[int]int, selectedInSlice, width int) string { if len(headers) == 0 { return "" } - columnWidths := processlistColumnWidths(headers, rows) - lines := []string{renderProcesslistRow(headers, headers, nil, headerStyle, columnWidths, width)} + columnWidths := tightColumnWidths(headers, rows) + lines := []string{renderTightRow(display, headers, headers, nil, 0, headerStyle, columnWidths, width)} for i, row := range rows { - style := lipgloss.NewStyle() - if i >= 0 && i < len(connections) { - conn := connections[i] - style = connectionRowStyle(conn, 0, i == selectedInSlice) - lines = append(lines, renderProcesslistRow(row, headers, &conn, style, columnWidths, width)) - continue + var conn *live.Connection + blockCount := 0 + base := lipgloss.NewStyle() + if i < len(connections) { + c := connections[i] + conn = &c + blockCount = counts[c.PID] + base = connectionRowStyle(c, blockCount, i == selectedInSlice) } - lines = append(lines, renderProcesslistRow(row, headers, nil, style, columnWidths, width)) + lines = append(lines, renderTightRow(display, row, headers, conn, blockCount, base, columnWidths, width)) } return strings.Join(lines, "\n") } -func processlistColumnWidths(headers []string, rows [][]string) []int { +func tightColumnWidths(headers []string, rows [][]string) []int { widths := make([]int, len(headers)) for i, header := range headers { widths[i] = ansi.StringWidth(header) @@ -799,16 +777,16 @@ func processlistColumnWidths(headers []string, rows [][]string) []int { return widths } -func renderProcesslistRow(cells []string, headers []string, conn *live.Connection, base lipgloss.Style, widths []int, width int) string { +func renderTightRow(display connectionDisplayPreset, cells, headers []string, conn *live.Connection, blockCount int, base lipgloss.Style, widths []int, width int) string { var line strings.Builder for i, cell := range cells { style := base - if conn != nil && i < len(headers) && headers[i] == "STATE" { - style = processlistStateStyleFor(style, conn.State) + if conn != nil { + style = connectionColumnStyleForDisplay(display, base, *conn, blockCount, i, headers) } if i < len(cells)-1 && i < len(widths) { cell = padCellToWidth(cell, widths[i]) - cell += processlistCellPadding(i) + cell += tightCellPadding(i) } line.WriteString(style.Render(cell)) } @@ -822,7 +800,7 @@ func padCellToWidth(text string, width int) string { return text } -func processlistCellPadding(index int) string { +func tightCellPadding(index int) string { if index == 0 { return " " } @@ -840,13 +818,6 @@ func connectionRowStyle(conn live.Connection, blockCount int, selected bool) lip return style } -func tableCellStyle(style lipgloss.Style, headers []string, col int) lipgloss.Style { - if col < len(headers)-1 { - return style.PaddingRight(2) - } - return style -} - func connectionColumnStyleForDisplay(display connectionDisplayPreset, style lipgloss.Style, conn live.Connection, blockCount int, col int, headers []string) lipgloss.Style { if display == connectionDisplayProcesslist { if col >= 0 && col < len(headers) && headers[col] == "STATE" { diff --git a/internal/connections/tui/table_test.go b/internal/connections/tui/table_test.go index a05ac05d..3b6cb95d 100644 --- a/internal/connections/tui/table_test.go +++ b/internal/connections/tui/table_test.go @@ -116,6 +116,40 @@ func TestRenderTableRendersNonEmptyList(t *testing.T) { c.Assert(rendered, qt.Contains, "SELECT * FROM widgets") } +func TestRenderTableKeepsColumnsTightWhenContentIsShort(t *testing.T) { + c := qt.New(t) + conns := []live.Connection{} + for _, app := range []string{"qa_demo_1", "qa_demo_2", "qa_demo_3", "qa_demo_4"} { + conns = append(conns, live.Connection{ + PID: 10, + State: "active", + ApplicationName: app, + QueryText: "SELECT pg_sleep(420)", + }) + } + list := live.NewConnectionList(tableRenderTestTime, conns, live.SortByTransactionStart) + + // With short, uniform content the table must be content-bound, not + // terminal-bound: widening the terminal must not widen the rows (the old + // lgtable layout spread columns to fill the whole width). Both widths keep + // the START column (>= startColumnMinWidth) so the column set matches. + maxDataWidth := func(w int) int { + rendered := stripANSI(renderTable(tableState{List: list, HasList: true, Width: w, Height: 12})) + widest := 0 + for _, line := range strings.Split(rendered, "\n") { + if strings.Contains(line, "qa_demo_") { + widest = max(widest, ansi.StringWidth(line)) + } + } + return widest + } + at150, at200 := maxDataWidth(150), maxDataWidth(200) + c.Assert(at200, qt.Equals, at150, + qt.Commentf("rows widened with the terminal (150=%d, 200=%d) — columns are being spread", at150, at200)) + c.Assert(at200 < 150, qt.IsTrue, + qt.Commentf("rows fill the terminal (width=%d) instead of staying content-tight", at200)) +} + func TestRenderTableInitialErrorDoesNotRepeatInFooter(t *testing.T) { c := qt.New(t) rendered := stripANSI(renderTable(tableState{