fix(cli): a Windows browser close ends the whole Chrome tree while its root is alive

This commit is contained in:
Miguel Angel Simon Sierra
2026-10-30 00:11:21 -04:00
parent 6c78dee993
commit f4ac1f168f
2 changed files with 42 additions and 3 deletions
@@ -1,9 +1,20 @@
import { spawn } from "node:child_process";
import { once } from "node:events";
import type { Browser, LaunchOptions, PuppeteerNode } from "puppeteer-core";
import { expect, it } from "vitest";
import { afterEach, expect, it, vi } from "vitest";
import { launchManagedBrowser } from "./launch.js";
const treeKill = vi.hoisted(() => vi.fn<(pid: number) => Promise<void>>());
vi.mock("../utils/processTree.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../utils/processTree.js")>()),
terminateWindowsProcessTree: treeKill,
}));
afterEach(() => {
treeKill.mockReset();
vi.restoreAllMocks();
});
it("kills a browser that acknowledged close but never exits, so every caller's close() returns", async () => {
const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: "pipe" });
const browser = { process: () => child, close: () => once(child, "exit") } as unknown as Browser;
@@ -18,3 +29,24 @@ it("kills a browser that acknowledged close but never exits, so every caller's c
child.kill("SIGKILL");
}
});
it("on Windows ends the browser's whole tree while its root still lives, since children outlive a killed root", async () => {
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: "pipe" });
let rootAliveAtTreeKill: boolean | undefined;
treeKill.mockImplementation(async () => {
rootAliveAtTreeKill = child.exitCode === null && child.signalCode === null;
});
const browser = { process: () => child, close: () => once(child, "exit") } as unknown as Browser;
const puppeteer = {
launch: async (_options: LaunchOptions) => browser,
} as unknown as PuppeteerNode;
try {
const launched = await launchManagedBrowser(puppeteer, { headless: true }, { diagnose: false });
await launched.close();
expect(treeKill).toHaveBeenCalledWith(child.pid);
expect(rootAliveAtTreeKill).toBe(true);
} finally {
child.kill("SIGKILL");
}
});
+9 -2
View File
@@ -17,7 +17,7 @@ import {
} from "./gpuPolicy.js";
import { normalizeErrorMessage, notePageCrash } from "../utils/errorMessage.js";
import { requestCliExit } from "../utils/commandResult.js";
import { processIsAlive } from "../utils/processTree.js";
import { processIsAlive, terminateWindowsProcessTree } from "../utils/processTree.js";
import { chromeDescendants, reapChromeTree } from "./chromeTree.js";
import { watchChromeTree, type ChromeWatchdog } from "./windowsChromeWatchdog.js";
@@ -290,6 +290,13 @@ function closeGracefully(browser: Browser): Promise<void> {
return (gracefulClose.get(browser) ?? (() => browser.close()))();
}
async function killBrowser(child: ChildProcess): Promise<void> {
if (process.platform === "win32" && child.pid !== undefined) {
await terminateWindowsProcessTree(child.pid).catch(() => undefined);
}
child.kill("SIGKILL");
}
async function closedWithoutKill(browser: Browser): Promise<boolean> {
const child = browser.process();
if (!child) throw new Error("A managed browser lost its owned process.");
@@ -309,7 +316,7 @@ async function closedWithoutKill(browser: Browser): Promise<boolean> {
grace,
]);
const clean = child.exitCode !== null || child.signalCode !== null;
if (!clean) child.kill("SIGKILL");
if (!clean) await killBrowser(child);
clearTimeout(timer);
const exitLimit = new Promise<never>((_, reject) => {
timer = setTimeout(