test(browse): unit coverage for the close() SIGKILL fallback

The wedge fix (capture the Chromium child before the close race,
SIGKILL on timeout) shipped without a test of the branch it added —
the coverage audit flagged it as the diff's one regression-gap. The
5s race window becomes an injectable closeRaceMs field, and four unit
tests pin: SIGKILL on hang, no SIGKILL on clean close, no SIGKILL on
an already-exited child, SIGKILL on a rejecting close.
This commit is contained in:
Garry Tan 2026-08-15 16:49:40 -07:00
parent 7b4b70babd
commit ec154bcb83
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
2 changed files with 74 additions and 2 deletions

View File

@ -797,6 +797,12 @@ export class BrowserManager {
this.consecutiveFailures = 0;
}
// How long close() waits for a graceful shutdown before falling back to
// SIGKILL (launched mode) or abandoning the context close (headed mode).
// A field, not a literal, so the SIGKILL fallback is unit-testable without
// a 5-second wait.
private closeRaceMs = 5000;
async close() {
// unref'd race timer: without unref, every successful close still pins
// the caller's event loop for the full window.
@ -811,7 +817,7 @@ export class BrowserManager {
if (this.browser) this.browser.removeAllListeners('disconnected');
await Promise.race([
this.context ? this.context.close() : Promise.resolve(),
raceTimeout(5000),
raceTimeout(this.closeRaceMs),
]).catch(() => {});
} else {
// Launched mode: close the browser we spawned.
@ -824,7 +830,7 @@ export class BrowserManager {
const child = this.browser.process?.();
const closed = await Promise.race([
this.browser.close().then(() => true as const),
raceTimeout(5000),
raceTimeout(this.closeRaceMs),
]).catch(() => false as const);
if (closed === false && child && child.exitCode === null && !child.killed) {
try { child.kill('SIGKILL'); } catch { /* already gone */ }

View File

@ -303,3 +303,69 @@ describe('stealth injected on every context-creation path', () => {
expect(sites.length).toBeGreaterThanOrEqual(2);
});
});
describe('close() launched-mode SIGKILL fallback', () => {
// The wedge this guards against: browser.close() hangs, the race times
// out, and pre-fix code nulled this.browser — ABANDONING a live Chromium
// whose sockets pinned the caller's event loop forever. The child must be
// captured before the race and SIGKILLed when graceful close loses.
type FakeChild = { exitCode: number | null; killed: boolean; kill: (sig: string) => void };
const makeCloseFakes = (closeBehavior: () => Promise<void>, child?: Partial<FakeChild>) => {
const kills: string[] = [];
const fakeChild: FakeChild = {
exitCode: null,
killed: false,
kill: (sig: string) => { kills.push(sig); },
...child,
};
const fakeBrowser = {
removeAllListeners: () => fakeBrowser,
process: () => fakeChild,
close: closeBehavior,
};
return { kills, fakeBrowser };
};
const managerWith = async (fakeBrowser: unknown) => {
const { BrowserManager } = await import('../src/browser-manager');
const bm = new BrowserManager();
const raw = bm as unknown as { browser: unknown; connectionMode: string; closeRaceMs: number };
raw.browser = fakeBrowser;
raw.connectionMode = 'launched';
raw.closeRaceMs = 20;
return { bm, raw };
};
it('SIGKILLs a live child when graceful close exceeds the race window', async () => {
const { kills, fakeBrowser } = makeCloseFakes(() => new Promise<void>(() => {}));
const { bm, raw } = await managerWith(fakeBrowser);
await bm.close();
expect(kills).toEqual(['SIGKILL']);
expect(raw.browser).toBeNull();
});
it('does not SIGKILL when graceful close finishes in time', async () => {
const { kills, fakeBrowser } = makeCloseFakes(async () => {});
const { bm, raw } = await managerWith(fakeBrowser);
await bm.close();
expect(kills).toEqual([]);
expect(raw.browser).toBeNull();
});
it('does not SIGKILL a child that already exited', async () => {
const { kills, fakeBrowser } = makeCloseFakes(
() => new Promise<void>(() => {}),
{ exitCode: 0 },
);
const { bm } = await managerWith(fakeBrowser);
await bm.close();
expect(kills).toEqual([]);
});
it('survives a rejecting close() and still SIGKILLs the live child', async () => {
const { kills, fakeBrowser } = makeCloseFakes(() => Promise.reject(new Error('target closed')));
const { bm } = await managerWith(fakeBrowser);
await bm.close();
expect(kills).toEqual(['SIGKILL']);
});
});