From ef70b7d88094f64c6d57d37191045a83bb7d834c Mon Sep 17 00:00:00 2001 From: Andrei Vershinin Date: Sat, 17 Oct 2026 14:08:13 +0000 Subject: [PATCH] fix(browse): name the busy tab when a command times out (#3247) A command that needs a stuck tab still waits out the CLI's 30s reply timeout, and the report should say why. It never ran: Bun rejects an AbortSignal.timeout() fetch with TimeoutError, the generic branch only matched AbortError, so a timeout fell through to "[browse] The operation timed out." - isFetchTimeout() accepts both names; the cookie-import branch uses it too. - commandTimeoutReport() reads /health: when `activeTab.responsive` is false it names the tab and points at `browse tab ` and `browse closetab `; a live daemon otherwise keeps the busy-not-restarting line and a dead one the plain timeout line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL --- browse/src/cli.ts | 48 ++++++++++++---- browse/test/command-timeout-report.test.ts | 66 ++++++++++++++++++++++ 2 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 browse/test/command-timeout-report.test.ts diff --git a/browse/src/cli.ts b/browse/src/cli.ts index 120f90f76..556947f0d 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -252,6 +252,13 @@ async function fetchDaemonTabCount(port: number): Promise { return typeof tabs === 'number' ? tabs : null; } +/** The active tab's id when /health reports its main thread as not + * responding (#3247), else null — including daemons that predate the field. */ +async function fetchBusyActiveTab(port: number): Promise { + const activeTab = (await fetchDaemonHealth(port))?.activeTab as { id?: unknown; responsive?: unknown } | null | undefined; + return activeTab?.responsive === false && typeof activeTab.id === 'number' ? activeTab.id : null; +} + // ─── Process Management ───────────────────────────────────────── async function killServer(pid: number): Promise { if (!isProcessAlive(pid)) return; @@ -1038,6 +1045,32 @@ export function extractTabId(args: string[]): { tabId: number | undefined; args: return { tabId, args: stripped }; } +/** Did this fetch fail because its AbortSignal fired? Bun rejects an + * AbortSignal.timeout() fetch with TimeoutError, not AbortError — matching + * only AbortError left the timeout reports below unreachable. */ +export function isFetchTimeout(err: unknown): boolean { + const name = (err as { name?: unknown } | null)?.name; + return name === 'AbortError' || name === 'TimeoutError'; +} + +/** + * What to print when a command's reply did not arrive within `timeoutMs`. + * #1781: a timeout on a heavy page usually means busy, not dead, so a live + * daemon is never restarted here. #3247: when /health names the active tab + * as not responding, say which tab and the commands that leave it without + * losing the session. + */ +export async function commandTimeoutReport(state: { pid: number; port: number } | null, timeoutMs: number): Promise { + const after = `[browse] Command timed out after ${timeoutMs / 1000}s`; + if (!state?.pid || !isProcessAlive(state.pid)) return [after]; + const busyTab = await fetchBusyActiveTab(state.port); + if (busyTab === null) return [`${after} (server still alive — busy, not restarting). Retry, or raise load.`]; + return [ + `${after}: tab ${busyTab} is not responding (its main thread is busy).`, + `[browse] The daemon, its other tabs and cookies still work: switch with 'browse tab ', close this one with 'browse closetab ${busyTab}', or retry once the page settles.`, + ]; +} + // ─── Command Dispatch ────────────────────────────────────────── export async function sendCommand(state: ServerState, command: string, args: string[], retries = 0): Promise { // Precedence: CLI --tab-id flag > BROWSE_TAB env var. @@ -1089,19 +1122,14 @@ export async function sendCommand(state: ServerState, command: string, args: str process.exit(1); } } catch (err: any) { - if (command === 'cookie-import-browser' && (['AbortError', 'TimeoutError'].includes(err.name) + if (command === 'cookie-import-browser' && (isFetchTimeout(err) || ['ECONNREFUSED', 'ECONNRESET'].includes(err.code) || err.message?.includes('fetch failed'))) { throw new Error('Cookie import response was lost or timed out. It may have partially completed; inspect the destination before retrying manually. The command was not replayed.'); } - if (err.name === 'AbortError') { - // #1781: a 30s timeout on a heavy page usually means busy, not dead. - // Don't kill a live server (that's what triggered the crash-loop) — report - // and exit so the user can retry rather than losing their (headed) window. - const ts = readState(); - const alive = ts?.pid ? isProcessAlive(ts.pid) : false; - console.error(alive - ? '[browse] Command timed out after 30s (server still alive — busy, not restarting). Retry, or raise load.' - : '[browse] Command timed out after 30s'); + if (isFetchTimeout(err)) { + // Report and exit so the user can retry rather than losing their + // (headed) window — killing a live server triggered the crash-loop. + for (const line of await commandTimeoutReport(readState(), timeoutMs)) console.error(line); process.exit(1); } // Connection error — server may have crashed, OR may just be busy. diff --git a/browse/test/command-timeout-report.test.ts b/browse/test/command-timeout-report.test.ts new file mode 100644 index 000000000..0505c0737 --- /dev/null +++ b/browse/test/command-timeout-report.test.ts @@ -0,0 +1,66 @@ +/** + * What the CLI prints when a command's reply never arrives (#1781, #3247). + * + * Bun rejects an AbortSignal.timeout() fetch with TimeoutError, so the old + * AbortError-only check skipped the busy report and printed "The operation + * timed out." instead. When the daemon's /health names its active tab as not + * responding, the report names that tab and the commands that leave it + * without a restart; a dead daemon gets the plain line. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { commandTimeoutReport, isFetchTimeout } from '../src/cli'; + +let servers: Array> = []; +afterEach(() => { for (const s of servers) s.stop(true); servers = []; }); + +/** A fake daemon whose /health answers `body`. */ +function fakeHealth(body: unknown): number { + const srv = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => Response.json(body) }); + servers.push(srv); + if (!srv.port) throw new Error('fake /health: no port'); + return srv.port; +} + +/** A pid that no longer runs: a child that has already exited. */ +function deadPid(): number { + return Bun.spawnSync([process.execPath, '-e', '']).pid; +} + +describe('isFetchTimeout', () => { + test('a real fetch whose AbortSignal.timeout fires counts as a timeout', async () => { + const silent = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => new Promise(() => {}) }); + servers.push(silent); + const err = await fetch(`http://127.0.0.1:${silent.port}/`, { signal: AbortSignal.timeout(50) }) + .then(() => null, (e: unknown) => e); + expect(isFetchTimeout(err)).toBe(true); + expect(isFetchTimeout(new TypeError('fetch failed'))).toBe(false); + }); +}); + +describe('commandTimeoutReport', () => { + test('names the busy tab and the way out, never a restart', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = (await commandTimeoutReport({ pid: process.pid, port }, 30_000)).join('\n'); + expect(out).toContain('tab 7'); + expect(out).toContain('closetab 7'); + expect(out).not.toContain('--force-restart'); + }); + + test('a live daemon with a responsive or unreported tab gets the busy-not-restarting line', async () => { + for (const body of [ + { status: 'healthy', activeTab: { id: 1, responsive: true } }, + { status: 'healthy' }, // daemon from before activeTab existed + ]) { + const out = (await commandTimeoutReport({ pid: process.pid, port: fakeHealth(body) }, 30_000)).join('\n'); + expect(out).toContain('still alive'); + expect(out).not.toContain('closetab'); + } + }); + + test('a dead daemon gets only the timeout line', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = await commandTimeoutReport({ pid: deadPid(), port }, 30_000); + expect(out).toHaveLength(1); + expect(out[0]).toContain('timed out after 30s'); + }); +});