From 0d64addfc19258884f0429edc091eb8ca232cab5 Mon Sep 17 00:00:00 2001 From: Ravi Pina Date: Tue, 16 Jun 2026 16:58:03 -0700 Subject: [PATCH] fix: align refresh board bar, humanize failure rows The done/failed rows padded the status glyph with two spaces while the active row used one, shifting the bar a column. Use one space everywhere. Replace the raw wrapped error on a failed row with a short classified reason (timeout, refused, host not found, auth, TLS) plus the dial target when present; the full error still prints in the command's trailing error block. --- internal/refreshui/board.go | 80 ++++++++++++++++++++++++++++++-- internal/refreshui/board_test.go | 49 ++++++++++++++++++- 2 files changed, 124 insertions(+), 5 deletions(-) diff --git a/internal/refreshui/board.go b/internal/refreshui/board.go index 699960d..3ebbc05 100644 --- a/internal/refreshui/board.go +++ b/internal/refreshui/board.go @@ -3,6 +3,7 @@ package refreshui import ( "fmt" "os" + "strings" "sync" "time" @@ -123,17 +124,19 @@ func (m *boardModel) View() string { b = append(b, '[') b = append(b, name...) b = append(b, ']', ' ') + // One space after the status glyph in every branch so the bar column + // lines up — the spinner and the ✔/✖ glyphs are each one cell wide. switch r.state { case rowDone: b = append(b, doneStyle.Render("✔")...) - b = append(b, ' ', ' ') + b = append(b, ' ') b = append(b, m.bar.ViewAs(1)...) b = append(b, fmt.Sprintf(" Started %dms", r.dur.Milliseconds())...) case rowFailed: b = append(b, failStyle.Render("✖")...) - b = append(b, ' ', ' ') + b = append(b, ' ') b = append(b, dimStyle.Render(dots(barWidth))...) - b = append(b, fmt.Sprintf(" Failed: %v", r.failErr)...) + b = append(b, fmt.Sprintf(" Failed: %s", friendlyError(r.failErr))...) default: // rowActive b = append(b, m.spin.View()...) b = append(b, ' ') @@ -151,6 +154,77 @@ func (m *boardModel) View() string { return string(b) } +// friendlyError reduces a wrapped refresh error to a short, operator-readable +// reason — the raw chain (e.g. `failed to fetch sites: ...: dial tcp HOST: i/o +// timeout`) is too long for a board row and gets truncated. The full error still +// prints in the command's trailing error block. When a dial target is present it +// is appended so the operator still knows which host failed. +func friendlyError(err error) string { + if err == nil { + return "unknown error" + } + msg := err.Error() + low := strings.ToLower(msg) + + var reason string + switch { + case strings.Contains(low, "deadline exceeded"), + strings.Contains(low, "timeout"), + strings.Contains(low, "timed out"): + reason = "connection timed out" + case strings.Contains(low, "connection refused"): + reason = "connection refused" + case strings.Contains(low, "no such host"): + reason = "host not found" + case strings.Contains(low, "no route to host"), + strings.Contains(low, "network is unreachable"): + reason = "network unreachable" + case strings.Contains(low, "tls"), strings.Contains(low, "certificate"): + reason = "TLS handshake failed" + case strings.Contains(low, "401"), + strings.Contains(low, "403"), + strings.Contains(low, "unauthorized"), + strings.Contains(low, "forbidden"), + strings.Contains(low, "invalid api key"), + strings.Contains(low, "authentication"): + reason = "authentication failed" + default: + reason = innermost(msg) + } + + if host := dialTarget(msg); host != "" { + return reason + " (" + host + ")" + } + return reason +} + +// dialTarget extracts the host:port from a Go dial error +// (`... dial tcp 10.0.0.1:443: i/o timeout`), or "" when absent. +func dialTarget(msg string) string { + const marker = "dial tcp " + i := strings.Index(strings.ToLower(msg), marker) + if i < 0 { + return "" + } + rest := msg[i+len(marker):] + if j := strings.Index(rest, ": "); j >= 0 { + rest = rest[:j] + } + return strings.TrimSpace(rest) +} + +// innermost returns the last `: `-delimited segment of a wrapped error — the +// root cause — for errors that don't match a known class. +func innermost(msg string) string { + parts := strings.Split(msg, ": ") + for i := len(parts) - 1; i >= 0; i-- { + if s := strings.TrimSpace(parts[i]); s != "" { + return s + } + } + return strings.TrimSpace(msg) +} + // dots returns n dot runes — the indeterminate stand-in for the bar before a // counted stage supplies a fraction. func dots(n int) string { diff --git a/internal/refreshui/board_test.go b/internal/refreshui/board_test.go index 14bc8d6..0d851ea 100644 --- a/internal/refreshui/board_test.go +++ b/internal/refreshui/board_test.go @@ -51,7 +51,7 @@ func TestBoardModelDoneAndError(t *testing.T) { m := newBoardModel([]string{"mist", "meraki"}) m.Update(doneMsg{label: "mist", dur: 952 * time.Millisecond}) - m.Update(errMsg{label: "meraki", err: errors.New("login timeout")}) + m.Update(errMsg{label: "meraki", err: errors.New("connection refused")}) if r := m.rows["mist"]; r.state != rowDone || r.dur != 952*time.Millisecond { t.Fatalf("mist row = %+v, want done 952ms", r) @@ -64,11 +64,56 @@ func TestBoardModelDoneAndError(t *testing.T) { if !strings.Contains(v, "Started 952ms") { t.Fatalf("view missing done summary:\n%s", v) } - if !strings.Contains(v, "Failed: login timeout") { + if !strings.Contains(v, "Failed: connection refused") { t.Fatalf("view missing error summary:\n%s", v) } } +func TestFriendlyError(t *testing.T) { + cases := []struct { + name string + err error + want string + }{ + { + name: "dial timeout keeps host", + err: errors.New(`failed to fetch sites: aruba-pina: POST /rest/login: Post "https://172.30.8.20:4343/rest/login": dial tcp 172.30.8.20:4343: i/o timeout`), + want: "connection timed out (172.30.8.20:4343)", + }, + {name: "refused", err: errors.New("dial tcp 10.0.0.1:443: connect: connection refused"), want: "connection refused (10.0.0.1:443)"}, + {name: "dns", err: errors.New(`Get "https://api.x.com": dial tcp: lookup api.x.com: no such host`), want: "host not found"}, + {name: "auth", err: errors.New("meraki: 401 Unauthorized"), want: "authentication failed"}, + {name: "unknown falls to innermost", err: errors.New("fetch sites: weird vendor explosion"), want: "weird vendor explosion"}, + {name: "nil", err: nil, want: "unknown error"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := friendlyError(tc.err); got != tc.want { + t.Fatalf("friendlyError = %q, want %q", got, tc.want) + } + }) + } +} + +// TestBoardRowsAlignAtBar guards the off-by-one fix: the bar (or dots) must +// start at the same column on active, done, and failed rows. The status glyphs +// (spinner, ✔, ✖) are all 3-byte runes, so equal byte offsets mean equal +// columns under the Ascii color profile (no escapes). +func TestBoardRowsAlignAtBar(t *testing.T) { + m := newBoardModel([]string{"api"}) + + m.Update(progressMsg{label: "api", done: 1, total: 2}) + active := strings.IndexRune(m.View(), '█') + m.Update(doneMsg{label: "api", dur: time.Second}) + done := strings.IndexRune(m.View(), '█') + m.Update(errMsg{label: "api", err: errors.New("x")}) + failed := strings.IndexRune(m.View(), '.') + + if active != done || active != failed { + t.Fatalf("bar columns differ: active=%d done=%d failed=%d\n%s", active, done, failed, m.View()) + } +} + func TestBoardModelLineCountStable(t *testing.T) { // The rendered line count must equal the API count regardless of state, so // bubbletea repaints in place instead of scrolling.