diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index b94afa9802f..1e4463ae94c 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -21,9 +21,17 @@ * `background-removal/manager.test.ts`) so we don't touch the real * `HOME` cache. */ -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join, relative, sep } from "node:path"; +import { basename, dirname, join, relative, sep } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { managedChromeVersion } from "./manager.js"; @@ -200,6 +208,7 @@ function installFsMocks({ interface InstallMockOptions { buildId: string; cacheDir: string; + unpack?: boolean; logger?: (prefix: string) => ((...args: unknown[]) => void) | undefined; } @@ -218,12 +227,14 @@ function installPuppeteerBrowsersMock( installImpl?: ( options: InstallMockOptions, ) => Promise<{ executablePath: string; path?: string }>; + unzip?: (archivePath: string, folderPath: string) => Promise; } = {}, ) { const impl = opts.installImpl ?? (async () => opts.installResult ?? { executablePath: HF_BINARY }); // Fixtures name the binary where it lands in HF_CACHE; install() really writes it under its own cacheDir. const stagedInstall = async (options: InstallMockOptions) => { + if (options.unpack === false) return join(options.cacheDir, "archive.zip"); const result = await impl(options); if (result.path || !result.executablePath.startsWith(HF_CACHE + sep)) return result; const rel = relative(HF_CACHE, result.executablePath); @@ -232,8 +243,15 @@ function installPuppeteerBrowsersMock( mockedPaths?.add(path).add(executablePath); return { executablePath, path }; }; + vi.doMock("./unzipArchive.js", () => ({ unzipArchive: opts.unzip ?? vi.fn(async () => true) })); vi.doMock("@puppeteer/browsers", () => ({ Browser: { CHROMEHEADLESSSHELL: "chrome-headless-shell" }, + Cache: class { + constructor(private readonly dir: string) {} + installationDir(browser: string, platform: string, buildId: string) { + return join(this.dir, browser, `${platform}-${buildId}`); + } + }, detectBrowserPlatform: () => opts.browserPlatform ?? "linux", getInstalledBrowsers: opts.installedInHfCacheError ? vi.fn().mockRejectedValue(opts.installedInHfCacheError) @@ -1484,6 +1502,65 @@ describe("downloadBrowser — too little disk for the browser", () => { rmSync(home, { recursive: true, force: true }); } }); + + it.each([ + { platform: "linux", unzipsFirst: true }, + { platform: "darwin", unzipsFirst: true }, + { platform: "win32", unzipsFirst: false }, + ])( + "on $platform, unzips the download itself first: $unzipsFirst", + async ({ platform, unzipsFirst }) => { + const home = useRealCacheHome(); + const { platform: origPlatform, arch: origArch } = process; + Object.defineProperty(process, "platform", { value: platform, configurable: true }); + Object.defineProperty(process, "arch", { value: "x64", configurable: true }); + const unzip = vi.fn(async () => true); + installPuppeteerBrowsersMock({ + unzip, + installImpl: async ({ cacheDir, buildId }) => + writeStagedInstall(cacheDir, buildId, "fresh"), + }); + try { + const { ensureBrowser } = await import("./manager.js"); + await ensureBrowser({ force: true }); + if (!unzipsFirst) return expect(unzip).not.toHaveBeenCalled(); + const [archive, into] = unzip.mock.calls[0] as unknown as [string, string]; + expect(into).toBe(stagedVersionDir(dirname(archive), managedChromeVersion())); + expect(basename(dirname(archive))).toMatch(/^\.chrome-staging-/); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + Object.defineProperty(process, "arch", { value: origArch, configurable: true }); + rmSync(home, { recursive: true, force: true }); + } + }, + ); + + it("a disk that fills mid-unzip fails the install at once and leaves no staged copy", async () => { + const home = useRealCacheHome(); + const { platform: origPlatform, arch: origArch } = process; + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + Object.defineProperty(process, "arch", { value: "x64", configurable: true }); + const install = vi.fn(async ({ cacheDir, buildId }: InstallMockOptions) => + writeStagedInstall(cacheDir, buildId, "fresh"), + ); + installPuppeteerBrowsersMock({ + installImpl: install, + unzip: async () => { + throw new Error("Extraction failed: the disk filled up while unpacking Chrome."); + }, + }); + try { + const { ensureBrowser } = await import("./manager.js"); + await expect(ensureBrowser({ force: true })).rejects.toThrow("the disk filled up"); + expect(install).not.toHaveBeenCalled(); + const root = join(home, ".cache", "hyperframes"); + expect(readdirSync(root).filter((name) => name.startsWith(".chrome-staging-"))).toEqual([]); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + Object.defineProperty(process, "arch", { value: origArch, configurable: true }); + rmSync(home, { recursive: true, force: true }); + } + }); }); // Regression guard for HF#2103: `hyperframes render` hung forever on macOS diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 7468b05e137..61b7f5b5301 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -23,8 +23,10 @@ import { import { getFreeDiskMb } from "../telemetry/system.js"; import { normalizeErrorMessage } from "../utils/errorMessage.js"; import { runRenderSetupWorker } from "../utils/cancellableProcess.js"; +import { unzipArchive } from "./unzipArchive.js"; type PuppeteerBrowsers = typeof import("@puppeteer/browsers"); +type InstallOptions = import("@puppeteer/browsers").InstallOptions; async function loadPuppeteerBrowsers(): Promise { try { @@ -707,7 +709,7 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise rmSync(stagingDir, { recursive: true, force: true }); // install() only debug-logs each failed unzip tool before trying the next one; keep those lines. let unzipLog: string[] = []; - const runInstall = () => { + const buildId = managedChromeVersion(); + const runInstall = async () => { unzipLog = []; - return install({ + const installOptions: InstallOptions & { unpack?: true } = { cacheDir: stagingDir, browser: Browser.CHROMEHEADLESSSHELL, - buildId: managedChromeVersion(), + buildId, platform, downloadProgressCallback: options?.onProgress, logger: (prefix) => { @@ -737,7 +740,14 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise { + // unzip -d creates only the last level; an empty folderPath would make install() report a broken install. + await mkdir(dirname(folderPath), { recursive: true }); + try { + await runCancellableProcess("unzip", ["-q", "-o", archivePath, "-d", folderPath], { + detached: false, + }); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return false; + if ((err as { status?: number }).status === UNZIP_DISK_FULL_STATUS) { + throw new Error( + "Extraction failed: the disk filled up while unpacking Chrome. Free disk space, then retry.", + { cause: err }, + ); + } + const said = (err as { stderr?: string }).stderr?.trim(); + throw new Error(`Extraction failed: ${said || (err as Error).message}`, { cause: err }); + } +} diff --git a/packages/cli/src/browser/unzipFallback.test.ts b/packages/cli/src/browser/unzipFallback.test.ts index 6e1d0999a00..8ec773e5a16 100644 --- a/packages/cli/src/browser/unzipFallback.test.ts +++ b/packages/cli/src/browser/unzipFallback.test.ts @@ -1,16 +1,27 @@ -import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { join, relative } from "node:path"; import { Browser, + Cache, computeExecutablePath, detectBrowserPlatform, install, } from "@puppeteer/browsers"; import AdmZip from "adm-zip"; import { afterEach, expect, it, vi } from "vitest"; +import { unzipArchive } from "./unzipArchive.js"; afterEach(() => { vi.unstubAllEnvs(); @@ -47,3 +58,110 @@ it.skipIf(process.platform === "win32")( } }, ); + +it.skipIf(process.platform === "win32")( + "unzips the download itself, and install() then finds the browser and drops the archive", + async () => { + const root = mkdtempSync(join(tmpdir(), "hf-unzip-own-")); + const platform = detectBrowserPlatform(); + if (!platform) throw new Error("unsupported test platform"); + const cacheDir = join(root, "cache"); + const options = { cacheDir, browser: Browser.CHROMEHEADLESSSHELL, buildId: "1.0.0", platform }; + const executablePath = computeExecutablePath(options); + const into = new Cache(cacheDir).installationDir(options.browser, platform, options.buildId); + const archive = new AdmZip(); + archive.addFile(relative(into, executablePath), Buffer.from("browser")); + const zipBytes = archive.toBuffer(); + const server = createServer((_request, response) => response.end(zipBytes)); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + try { + const baseUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; + + const archivePath = await install({ ...options, baseUrl, unpack: false }); + expect(await unzipArchive(archivePath, into)).toBe(true); + expect(existsSync(executablePath)).toBe(true); + const installed = await install({ ...options, baseUrl }); + + expect(installed.executablePath).toBe(executablePath); + expect(existsSync(archivePath)).toBe(false); + } finally { + server.close(); + rmSync(root, { recursive: true, force: true }); + } + }, +); + +it.skipIf(process.platform === "win32")( + "a full disk fails the unzip at once instead of waiting on its Continue? question", + async () => { + const root = mkdtempSync(join(tmpdir(), "hf-unzip-full-")); + const bin = join(root, "bin"); + mkdirSync(bin); + // What unzip does on a write error: ask, wait for an answer on its input, then give up. + writeFileSync( + join(bin, "unzip"), + "#!/bin/sh\nprintf 'write error (disk full?). Continue? (y/n/^C) ' >&2\nread answer\nexit 50\n", + ); + chmodSync(join(bin, "unzip"), 0o755); + vi.stubEnv("PATH", `${bin}:/usr/bin:/bin`); + try { + await expect(unzipArchive(join(root, "a.zip"), join(root, "out"))).rejects.toThrow( + "Extraction failed: the disk filled up while unpacking Chrome. Free disk space, then retry.", + ); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, + 5_000, +); + +it.skipIf(process.platform === "win32")( + "runs unzip in the caller's process group, so cancelling the setup worker stops it", + async () => { + const root = mkdtempSync(join(tmpdir(), "hf-unzip-group-")); + const bin = join(root, "bin"); + mkdirSync(bin); + writeFileSync(join(bin, "unzip"), `#!/bin/sh\nps -o pgid= -p $$ > "${join(root, "pgid")}"\n`); + chmodSync(join(bin, "unzip"), 0o755); + vi.stubEnv("PATH", `${bin}:/usr/bin:/bin`); + try { + expect(await unzipArchive(join(root, "a.zip"), join(root, "out"))).toBe(true); + const callerGroup = execFileSync("ps", ["-o", "pgid=", "-p", String(process.pid)], { + encoding: "utf8", + }); + expect(readFileSync(join(root, "pgid"), "utf8").trim()).toBe(callerGroup.trim()); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, +); + +it.skipIf(process.platform === "win32")( + "unzips into a folder whose parents do not exist yet", + async () => { + const root = mkdtempSync(join(tmpdir(), "hf-unzip-deep-")); + const archive = new AdmZip(); + archive.addFile("chrome", Buffer.from("browser")); + archive.writeZip(join(root, "a.zip")); + try { + expect(await unzipArchive(join(root, "a.zip"), join(root, "one", "two"))).toBe(true); + expect(existsSync(join(root, "one", "two", "chrome"))).toBe(true); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, +); + +it.skipIf(process.platform === "win32")( + "says there is no unzip command, and leaves no empty folder for install() to mistake for a broken install", + async () => { + const root = mkdtempSync(join(tmpdir(), "hf-unzip-none-")); + vi.stubEnv("PATH", join(root, "empty-bin")); + try { + expect(await unzipArchive(join(root, "a.zip"), join(root, "one", "two"))).toBe(false); + expect(existsSync(join(root, "one", "two"))).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, +); diff --git a/packages/cli/src/utils/cancellableProcess.test.ts b/packages/cli/src/utils/cancellableProcess.test.ts index 92a937c8b74..5161085b14b 100644 --- a/packages/cli/src/utils/cancellableProcess.test.ts +++ b/packages/cli/src/utils/cancellableProcess.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from "node:child_process"; import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -7,6 +8,10 @@ import { testProcessIsAlive, waitForTestCondition } from "./processTestUtils.js" const IS_POSIX = process.platform !== "win32"; +function processGroupOf(pid: number): string { + return execFileSync("ps", ["-o", "pgid=", "-p", String(pid)], { encoding: "utf8" }).trim(); +} + describe.skipIf(!IS_POSIX)("cancellable process tree teardown", () => { it("reaps a SIGTERM-resistant grandchild after the setup root exits", async () => { const testDir = mkdtempSync(join(tmpdir(), "hyperframes-cancellable-process-")); @@ -78,6 +83,33 @@ describe.skipIf(!IS_POSIX)("cancellable process tree teardown", () => { rmSync(testDir, { recursive: true, force: true }); } }, 5_000); + + it("keeps a child it does not detach in the caller's process group, and a cancel still stops it", async () => { + const controller = new AbortController(); + const abortReason = new Error("cancel unzip"); + let childPid: number | undefined; + try { + const run = runCancellableProcess( + process.execPath, + ["--eval", "setTimeout(() => undefined, 10_000)"], + { + signal: controller.signal, + detached: false, + onSpawn: (pid) => { + childPid = pid; + }, + }, + ); + expect(processGroupOf(childPid!)).toBe(processGroupOf(process.pid)); + + controller.abort(abortReason); + await expect(run).rejects.toBe(abortReason); + + expect(testProcessIsAlive(childPid!)).toBe(false); + } finally { + if (childPid && testProcessIsAlive(childPid)) process.kill(childPid, "SIGKILL"); + } + }, 5_000); }); describe("render setup worker failure", () => { diff --git a/packages/cli/src/utils/cancellableProcess.ts b/packages/cli/src/utils/cancellableProcess.ts index 6916e15d28a..515eb01a147 100644 --- a/packages/cli/src/utils/cancellableProcess.ts +++ b/packages/cli/src/utils/cancellableProcess.ts @@ -14,6 +14,7 @@ export interface CancellableProcessOptions { maxBufferBytes?: number; cwd?: string; env?: NodeJS.ProcessEnv; + detached?: boolean; onSpawn?: (pid: number) => void; } @@ -56,6 +57,7 @@ export function runCancellableProcess( const signal = options.signal; signal?.throwIfAborted(); + const detached = options.detached ?? process.platform !== "win32"; return new Promise((resolve, reject) => { let child: ChildProcess; try { @@ -64,7 +66,7 @@ export function runCancellableProcess( env: options.env, stdio: ["ignore", "pipe", "pipe"], windowsHide: true, - detached: process.platform !== "win32", + detached, }; child = spawn(command, [...args], spawnOptions); } catch (error) { @@ -102,7 +104,7 @@ export function runCancellableProcess( terminationCode = code; terminationSignal = "SIGTERM"; if (processTreePid) { - terminationTask = terminateProcessTree(processTreePid, { child, processGroup: true }); + terminationTask = terminateProcessTree(processTreePid, { child, processGroup: detached }); void terminationTask.catch(() => undefined); } };