mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-17 10:25:33 +02:00
fix(browse): daemon resilience on loaded machines — Bun conn errors, stop/restart flush, startup + git-root budgets
Four load-sensitivity fixes in the daemon lifecycle:
- sendCommand only recognized Node's ECONNREFUSED/ECONNRESET; the compiled
CLI runs on Bun, which reports 'ConnectionRefused'/'ConnectionClosed'
("Unable to connect..."), so daemon crashes leaked the raw error and
exited 1 instead of entering the busy-check/restart path. Match both.
- stop/restart called shutdown() inline, which exits before the HTTP
response flushes — the CLI saw a dropped socket (and would now
crash-retry a fresh daemon just to stop it). Defer shutdown ~100ms so
the 200 lands first.
- Non-CI POSIX startup budget raised 8s -> 15s (cold Chromium measured
~5.7s at load avg 10; load 12+ blew the old budget while the detached
daemon was still booting).
- getGitRoot's 2s git rev-parse timeout returned null under load (6.3s
spikes measured), scattering state files across cwds into split-brain
daemons. Raise to 8s, still bounded.
Contributed by @mplatts (PR #1732).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
67a96fed9b
commit
a6c94c7feb
+18
-2
@@ -36,7 +36,13 @@ const IS_WINDOWS = process.platform === 'win32';
|
|||||||
* falls back to the platform default. Pure + exported for tests.
|
* falls back to the platform default. Pure + exported for tests.
|
||||||
*/
|
*/
|
||||||
export function resolveStartTimeout(env: NodeJS.ProcessEnv = process.env): number {
|
export function resolveStartTimeout(env: NodeJS.ProcessEnv = process.env): number {
|
||||||
const platformDefault = IS_WINDOWS ? 15000 : (env.CI ? 30000 : 8000); // Node+Chromium takes longer on Windows
|
// Cold Chromium launch measured ~5.7s at load avg 10 on a dev machine running
|
||||||
|
// many servers; at load 12+ it exceeds the old 8s budget, so the CLI gave up
|
||||||
|
// while the (detached) daemon was still booting → "Server failed to start
|
||||||
|
// within 8s". 15s matches the Windows budget and gives real headroom; the poll
|
||||||
|
// loop returns the instant the daemon is healthy, so this only costs time in a
|
||||||
|
// genuine-failure case.
|
||||||
|
const platformDefault = IS_WINDOWS ? 15000 : (env.CI ? 30000 : 15000); // Node+Chromium takes longer on Windows
|
||||||
const override = parseInt(env.BROWSE_START_TIMEOUT || '', 10);
|
const override = parseInt(env.BROWSE_START_TIMEOUT || '', 10);
|
||||||
return Number.isFinite(override) && override > 0 ? override : platformDefault;
|
return Number.isFinite(override) && override > 0 ? override : platformDefault;
|
||||||
}
|
}
|
||||||
@@ -614,7 +620,17 @@ async function sendCommand(state: ServerState, command: string, args: string[],
|
|||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
// Connection error — server may have crashed, OR may just be busy.
|
// Connection error — server may have crashed, OR may just be busy.
|
||||||
if (err.code === 'ECONNREFUSED' || err.code === 'ECONNRESET' || err.message?.includes('fetch failed')) {
|
// The compiled CLI runs on Bun, whose fetch reports a refused/dropped
|
||||||
|
// socket as err.code 'ConnectionRefused' / 'ConnectionClosed' (message
|
||||||
|
// "Unable to connect. Is the computer able to access the url?"), NOT Node's
|
||||||
|
// ECONNREFUSED/ECONNRESET. Match both, or daemon crashes leak the raw Bun
|
||||||
|
// error and exit 1 instead of triggering the busy-check/restart below.
|
||||||
|
const isConnError =
|
||||||
|
err.code === 'ECONNREFUSED' || err.code === 'ECONNRESET' ||
|
||||||
|
err.code === 'ConnectionRefused' || err.code === 'ConnectionClosed' ||
|
||||||
|
err.message?.includes('fetch failed') ||
|
||||||
|
err.message?.includes('Unable to connect');
|
||||||
|
if (isConnError) {
|
||||||
const oldState = readState();
|
const oldState = readState();
|
||||||
// #1781 busy-vs-dead: a single-threaded daemon under beacon/extension load
|
// #1781 busy-vs-dead: a single-threaded daemon under beacon/extension load
|
||||||
// can briefly stop answering HTTP while still alive. Before declaring a
|
// can briefly stop answering HTTP while still alive. Before declaring a
|
||||||
|
|||||||
@@ -34,7 +34,12 @@ export function getGitRoot(): string | null {
|
|||||||
const proc = Bun.spawnSync(['git', 'rev-parse', '--show-toplevel'], {
|
const proc = Bun.spawnSync(['git', 'rev-parse', '--show-toplevel'], {
|
||||||
stdout: 'pipe',
|
stdout: 'pipe',
|
||||||
stderr: 'pipe',
|
stderr: 'pipe',
|
||||||
timeout: 2_000, // Don't hang if .git is broken
|
// Raised from 2s: under heavy machine load `git rev-parse` routinely
|
||||||
|
// takes >2s (measured 6.3s spikes). Timing out here returns null →
|
||||||
|
// resolveConfig falls back to process.cwd() → state files scatter across
|
||||||
|
// cwds (split-brain daemons; `goto` and `url` hit different servers). 8s
|
||||||
|
// still bounds a genuinely broken .git from hanging the CLI forever.
|
||||||
|
timeout: 8_000,
|
||||||
});
|
});
|
||||||
if (proc.exitCode !== 0) return null;
|
if (proc.exitCode !== 0) return null;
|
||||||
return proc.stdout.toString().trim() || null;
|
return proc.stdout.toString().trim() || null;
|
||||||
|
|||||||
@@ -421,15 +421,25 @@ export async function handleMetaCommand(
|
|||||||
}
|
}
|
||||||
|
|
||||||
case 'stop': {
|
case 'stop': {
|
||||||
await shutdown();
|
// Defer shutdown so the response flushes before process.exit() (same
|
||||||
|
// reason as 'restart' below). Otherwise the CLI sees a dropped socket;
|
||||||
|
// and now that connection-loss triggers the crash-retry path, that would
|
||||||
|
// resurrect a fresh daemon only to stop it again. Send the 200, then exit.
|
||||||
|
setTimeout(() => { void shutdown(); }, 100);
|
||||||
return 'Server stopped';
|
return 'Server stopped';
|
||||||
}
|
}
|
||||||
|
|
||||||
case 'restart': {
|
case 'restart': {
|
||||||
// Signal that we want a restart — the CLI will detect exit and restart
|
// Signal that we want a restart — the CLI will detect exit and restart.
|
||||||
console.log('[browse] Restart requested. Exiting for CLI to restart.');
|
console.log('[browse] Restart requested. Exiting for CLI to restart.');
|
||||||
await shutdown();
|
// Defer shutdown one tick so this HTTP response actually flushes before
|
||||||
return 'Restarting...';
|
// process.exit(). shutdown() exits inline (server.ts), so the old
|
||||||
|
// `await shutdown(); return 'Restarting...'` never sent a response — the
|
||||||
|
// CLI saw a dropped socket and `browse restart` errored out. The daemon
|
||||||
|
// now exits ~100ms after the CLI gets its 200; the next browse command
|
||||||
|
// lazily cold-starts a fresh one.
|
||||||
|
setTimeout(() => { void shutdown(); }, 100);
|
||||||
|
return 'Restarting... (daemon exiting; next browse command starts a fresh one)';
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Visual ────────────────────────────────────────
|
// ─── Visual ────────────────────────────────────────
|
||||||
|
|||||||
@@ -884,7 +884,10 @@ describe('CLI lifecycle', () => {
|
|||||||
cliEnv.BROWSE_STATE_FILE = stateFile;
|
cliEnv.BROWSE_STATE_FILE = stateFile;
|
||||||
const result = await new Promise<{ code: number; stdout: string; stderr: string }>((resolve) => {
|
const result = await new Promise<{ code: number; stdout: string; stderr: string }>((resolve) => {
|
||||||
const proc = spawn('bun', ['run', cliPath, 'status'], {
|
const proc = spawn('bun', ['run', cliPath, 'status'], {
|
||||||
timeout: 15000,
|
// Must exceed the CLI's startup budget (resolveStartTimeout, 15s
|
||||||
|
// non-CI POSIX) or a slow cold boot under full-suite load gets the
|
||||||
|
// child killed at the exact moment the CLI would have succeeded.
|
||||||
|
timeout: 18000,
|
||||||
env: cliEnv,
|
env: cliEnv,
|
||||||
});
|
});
|
||||||
let stdout = '';
|
let stdout = '';
|
||||||
|
|||||||
Reference in New Issue
Block a user