mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
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 <id>` and `browse closetab <id>`; 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL
This commit is contained in:
1 parent
d010d7b243
commit
ef70b7d880
2 files changed
+104
-10
No files matched your search
+38
-10
@@ -252,6 +252,13 @@ async function fetchDaemonTabCount(port: number): Promise<number | null> {
|
||||
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<number | null> {
|
||||
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<void> {
|
||||
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<string[]> {
|
||||
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 <id>', 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<void> {
|
||||
// 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.
|
||||
|
||||
@@ -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<ReturnType<typeof Bun.serve>> = [];
|
||||
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<Response>(() => {}) });
|
||||
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');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user