This commit is contained in:
TTmo123 2026-07-14 19:16:22 -07:00 committed by GitHub
commit 115a3f8acc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 110 additions and 0 deletions

View File

@ -585,6 +585,15 @@ async function sendCommand(state: ServerState, command: string, args: string[],
}
// Connection error — server may have crashed, OR may just be busy.
if (err.code === 'ECONNREFUSED' || err.code === 'ECONNRESET' || err.message?.includes('fetch failed')) {
// User-initiated shutdown (stop / restart) intentionally closes the daemon.
// Both meta-commands.ts:423 ('stop') and :428 ('restart') call shutdown(),
// which closes the HTTP connection before the response body reaches the
// client. ECONNRESET here is the expected exit signal, not a crash.
// 'command' is already in closure from sendCommand's signature — no new
// param passing.
if (command === 'stop' || command === 'restart') {
process.exit(0);
}
const oldState = readState();
// #1781 busy-vs-dead: a single-threaded daemon under beacon/extension load
// can briefly stop answering HTTP while still alive. Before declaring a

View File

@ -0,0 +1,101 @@
/**
* sendCommand ECONNRESET short-circuit for user-initiated shutdown.
*
* Validates that browse stop / browse restart do NOT trigger the
* "Server crashed twice in a row" warning when the daemon intentionally
* closes the HTTP connection. The regression guard confirms the
* short-circuit only applies to stop/restart non-shutdown commands
* still hit the existing crash-retry logic.
*
* We exercise the actual binary because sendCommand is an internal
* module-level function and cannot be unit-mocked against a real
* daemon fetch endpoint without re-architecting cli.ts. The test
* gracefully degrades when the binary is not built
* (describe.skipIf(!HAS_BINARY)) so CI shards without a dist/ still
* pass.
*/
import { describe, test, expect } from 'bun:test';
import { spawnSync } from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
const ROOT = path.resolve(import.meta.dir, '..');
const BROWSE_BIN = path.join(
ROOT,
'browse',
'dist',
process.platform === 'win32' ? 'browse.exe' : 'browse',
);
const HAS_BINARY = (() => {
try {
return fs.existsSync(BROWSE_BIN);
} catch {
return false;
}
})();
describe.skipIf(!HAS_BINARY)('browse: user-initiated shutdown skips crash-retry', () => {
test('stop command on ECONNRESET does not print crash warning', () => {
// Ensure daemon is up; status is idempotent and will start one if needed.
spawnSync(BROWSE_BIN, ['status'], { encoding: 'utf-8', timeout: 10000 });
const result = spawnSync(BROWSE_BIN, ['stop'], {
encoding: 'utf-8',
timeout: 10000,
cwd: ROOT,
});
expect(result.status).toBe(0);
expect(result.stderr).not.toContain('crashed');
expect(result.stderr).not.toContain('aborting');
expect(result.stderr).not.toContain('fetch failed');
});
test('restart command on ECONNRESET does not print crash warning', () => {
spawnSync(BROWSE_BIN, ['status'], { encoding: 'utf-8', timeout: 10000 });
const result = spawnSync(BROWSE_BIN, ['restart'], {
encoding: 'utf-8',
timeout: 15000,
cwd: ROOT,
});
expect(result.status).toBe(0);
expect(result.stderr).not.toContain('crashed');
expect(result.stderr).not.toContain('aborting');
});
});
describe('browse: sendCommand short-circuit regression guard', () => {
// Code-shape test: catches a refactor that accidentally widens the
// short-circuit to all commands (which would silently skip crash-retry
// for genuine daemon failures during goto/status/etc.).
test('guard is scoped to stop + restart only', () => {
const cliPath = path.join(ROOT, 'browse', 'src', 'cli.ts');
const cliSrc = fs.readFileSync(cliPath, 'utf-8');
// The guard must check both 'stop' AND 'restart'
expect(cliSrc).toMatch(
/if\s*\(\s*command\s*===\s*['"]stop['"]\s*\|\|\s*command\s*===\s*['"]restart['"]\s*\)/,
);
// The guard must call process.exit(0) (not throw — throw would be
// caught by main().catch() and print "[browse] fetch failed")
expect(cliSrc).toMatch(/process\.exit\(0\)/);
// The guard must live inside the existing ECONNRESET handler, not
// replace it
expect(cliSrc).toContain('ECONNREFUSED');
expect(cliSrc).toContain('ECONNRESET');
});
test('guard preserves original crash-retry path for non-shutdown commands', () => {
const cliPath = path.join(ROOT, 'browse', 'src', 'cli.ts');
const cliSrc = fs.readFileSync(cliPath, 'utf-8');
// The "Server crashed twice in a row" message must still exist —
// removing it would silently degrade the genuine-crash reporting.
expect(cliSrc).toContain('Server crashed twice in a row');
// And it must be reached when retries >= 1
expect(cliSrc).toMatch(/retries\s*>=\s*1.*aborting/);
});
});