From 6bd9a4819ecef9a6698e93cd31f0e1a65115cd82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 24 Oct 2026 11:09:17 -0400 Subject: [PATCH 1/4] fix(cli): a Chrome download that stops sending is abandoned, retried, and releases the lock --- packages/cli/src/browser/manager.test.ts | 81 ++++++++++++++++++++++ packages/cli/src/browser/manager.ts | 88 +++++++++++++++++++++--- 2 files changed, 161 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index afac197bb81..76e85691b48 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -30,6 +30,8 @@ import { rmSync, writeFileSync, } from "node:fs"; +import { request } from "node:http"; +import { createServer, type Socket } from "node:net"; import { tmpdir } from "node:os"; import { basename, dirname, join, relative, sep } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -228,6 +230,7 @@ function installPuppeteerBrowsersMock( options: InstallMockOptions, ) => Promise<{ executablePath: string; path?: string }>; unzip?: (archivePath: string, folderPath: string) => Promise; + downloadUrl?: string; } = {}, ) { const impl = @@ -262,6 +265,7 @@ function installPuppeteerBrowsersMock( })), ), install: vi.fn().mockImplementation(stagedInstall), + ...(opts.downloadUrl && { getDownloadUrl: () => opts.downloadUrl }), })); } @@ -1433,6 +1437,83 @@ describe("downloadBrowser — install failure surfaces HYPERFRAMES_BROWSER_PATH ); }); +describe("downloadBrowser — a download that stops sending", () => { + const sockets: Socket[] = []; + + beforeEach(() => { + vi.resetModules(); + delete process.env["HYPERFRAMES_BROWSER_PATH"]; + delete process.env["PRODUCER_HEADLESS_SHELL_PATH"]; + installChildProcessMocks(); + }); + + afterEach(() => { + for (const socket of sockets.splice(0)) socket.destroy(); + vi.restoreAllMocks(); + vi.doUnmock("node:fs"); + vi.doUnmock("node:os"); + vi.doUnmock("node:child_process"); + vi.doUnmock("@puppeteer/browsers"); + }); + + it("gives up with the host named, retries, releases the install lock and points at HYPERFRAMES_BROWSER_PATH", async () => { + const server = createServer((socket) => sockets.push(socket)); + await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); + const { port } = server.address() as { port: number }; + const host = `127.0.0.1:${port}`; + installFsMocks({ existing: new Set([CACHE_ROOT]) }); + let attempts = 0; + installPuppeteerBrowsersMock({ + installedInHfCache: [], + downloadUrl: `http://${host}/chrome-headless-shell.zip`, + installImpl: () => { + attempts += 1; + return new Promise((_resolve, reject) => { + request({ host: "127.0.0.1", port, path: "/chrome-headless-shell.zip" }) + .on("error", reject) + .end(); + }); + }, + }); + const { ensureBrowser } = await import("./manager.js"); + + try { + const error = await ensureBrowser({ downloadStallMs: 100 }).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + expect(message).toContain(`The Chrome download from ${host} stalled: no data for 0.1s`); + expect(message).toContain("HYPERFRAMES_BROWSER_PATH"); + expect(attempts).toBe(3); + expect(mockedPaths?.has(HF_LOCK)).toBe(false); + } finally { + for (const socket of sockets) socket.destroy(); + await new Promise((closed) => server.close(closed)); + } + }); + + it("does not time out a download that keeps making progress", async () => { + installFsMocks({ existing: new Set([CACHE_ROOT]) }); + installPuppeteerBrowsersMock({ + installedInHfCache: [], + installImpl: async (options) => { + const progress = (options as { downloadProgressCallback?: (d: number, t: number) => void }) + .downloadProgressCallback; + for (let bytes = 1; bytes <= 5; bytes++) { + await new Promise((tick) => setTimeout(tick, 60)); + progress?.(bytes, 5); + } + return { executablePath: HF_BINARY }; + }, + }); + const { ensureBrowser } = await import("./manager.js"); + + await expect(ensureBrowser({ downloadStallMs: 150 })).resolves.toMatchObject({ + source: "download", + }); + }); +}); + describe("downloadBrowser — too little disk for the browser", () => { beforeEach(() => { vi.resetModules(); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 61b7f5b5301..9ee452a1a59 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -229,6 +229,7 @@ export interface BrowserResult { export interface EnsureBrowserOptions { onProgress?: (downloadedBytes: number, totalBytes: number) => void; + downloadStallMs?: number; // Re-download the managed build even when cached; only its version dir is replaced. force?: boolean; // Always resolve to OUR pinned `CHROME_VERSION` build (cached, or freshly @@ -695,6 +696,55 @@ function wrapDownloadFailureWithBrowserPathHint( return new Error(message, { cause: cause instanceof Error ? cause : undefined }); } +const DOWNLOAD_STALL_MS = 60_000; +const DOWNLOAD_ATTEMPTS = 3; + +class BrowserDownloadStalledError extends Error {} + +type ProgressCallback = (downloadedBytes: number, totalBytes: number) => void; + +// install() takes no abort signal: a stalled transfer is abandoned, so the command and its lock stop waiting on it. +function withDownloadStallTimeout( + start: (onProgress: ProgressCallback) => Promise, + { stallMs, host }: { stallMs: number; host: string }, +): Promise { + return new Promise((resolve, reject) => { + let timer: NodeJS.Timeout | undefined; + const arm = () => { + clearTimeout(timer); + timer = setTimeout( + () => + reject( + new BrowserDownloadStalledError( + `The Chrome download from ${host} stalled: no data for ${stallMs / 1000}s.`, + ), + ), + stallMs, + ); + timer.unref?.(); + }; + arm(); + start((downloaded, total) => { + if (total > 0 && downloaded >= total) clearTimeout(timer); + else arm(); + }) + .then(resolve, reject) + .finally(() => clearTimeout(timer)); + }); +} + +async function retryOnStall(run: () => Promise, clear: () => void): Promise { + for (let attempt = 1; ; attempt++) { + try { + return await run(); + } catch (err) { + if (!(err instanceof BrowserDownloadStalledError) || attempt >= DOWNLOAD_ATTEMPTS) throw err; + console.warn(`[hyperframes] ${err.message} Retrying (${attempt}/${DOWNLOAD_ATTEMPTS - 1}).`); + clear(); + } + } +} + async function downloadBrowser(options?: EnsureBrowserOptions): Promise { if (isLinuxArm()) { return ensureLinuxArmBrowser(options); @@ -709,7 +759,8 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise { unzipLog = []; const installOptions: InstallOptions & { unpack?: true } = { @@ -729,7 +787,6 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise { const debug = debuglog(prefix); const isUnzip = prefix === "puppeteer:browsers:fileUtil"; @@ -741,20 +798,35 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise(call: (onProgress: ProgressCallback) => Promise) => + withDownloadStallTimeout( + (onProgress) => + call((downloaded, total) => { + options?.onProgress?.(downloaded, total); + onProgress(downloaded, total); + }), + { stallMs: options?.downloadStallMs ?? DOWNLOAD_STALL_MS, host: downloadHost }, + ); const unzipMayAskOnInput = process.platform !== "win32"; if (unzipMayAskOnInput) { - const archive = await install({ ...installOptions, unpack: false }); + const archive = await guarded((cb) => + install({ ...installOptions, downloadProgressCallback: cb, unpack: false }), + ); const into = new Cache(stagingDir).installationDir(installOptions.browser, platform, buildId); await unzipArchive(archive, into); } - return install(installOptions); + return guarded((cb) => install({ ...installOptions, downloadProgressCallback: cb })); }; try { - const staged = await installWithCorruptArchiveRecovery(runInstall, clearStaging, (err) => - console.warn( - `[hyperframes] Downloaded browser archive was corrupt (${normalizeErrorMessage(err)}); re-downloading.`, - ), + const staged = await retryOnStall( + () => + installWithCorruptArchiveRecovery(runInstall, clearStaging, (err) => + console.warn( + `[hyperframes] Downloaded browser archive was corrupt (${normalizeErrorMessage(err)}); re-downloading.`, + ), + ), + clearStaging, ); assertExecutableUnpacked(stagingDir, staged.executablePath); unzipLog = []; From d0363e2804dfb2a0b9716f7e8eddb8598b5d7b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 24 Oct 2026 11:30:40 -0400 Subject: [PATCH 2/4] fix(cli): a stalled Chrome download is aborted at the socket so the process can exit --- .../cli/src/browser/downloadStall.fixture.ts | 14 +++ .../src/browser/downloadStall.process.test.ts | 49 ++++++++++ packages/cli/src/browser/manager.ts | 89 ++++++++++++++----- 3 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 packages/cli/src/browser/downloadStall.fixture.ts create mode 100644 packages/cli/src/browser/downloadStall.process.test.ts diff --git a/packages/cli/src/browser/downloadStall.fixture.ts b/packages/cli/src/browser/downloadStall.fixture.ts new file mode 100644 index 00000000000..9d322a05e47 --- /dev/null +++ b/packages/cli/src/browser/downloadStall.fixture.ts @@ -0,0 +1,14 @@ +// fallow-ignore-file unused-file +import { ensureBrowser } from "./manager.js"; + +const [baseUrl] = process.argv.slice(2); +try { + await ensureBrowser({ + preferManagedChrome: true, + downloadStallMs: 200, + downloadBaseUrl: baseUrl, + }); +} catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exitCode = 1; +} diff --git a/packages/cli/src/browser/downloadStall.process.test.ts b/packages/cli/src/browser/downloadStall.process.test.ts new file mode 100644 index 00000000000..f985bef5549 --- /dev/null +++ b/packages/cli/src/browser/downloadStall.process.test.ts @@ -0,0 +1,49 @@ +import { spawn } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { createServer, type Socket } from "node:net"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, expect, it } from "vitest"; + +const fixturePath = join(dirname(fileURLToPath(import.meta.url)), "downloadStall.fixture.ts"); +const cleanups: (() => void)[] = []; + +afterEach(() => { + for (const cleanup of cleanups.splice(0)) cleanup(); +}); + +it("exits with code 1 when the download host accepts the connection and never answers", async () => { + const sockets: Socket[] = []; + const server = createServer((socket) => sockets.push(socket)); + await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); + const { port } = server.address() as { port: number }; + const home = mkdtempSync(join(tmpdir(), "hf-download-stall-")); + const child = spawn( + process.execPath, + ["--import", "tsx", fixturePath, `http://127.0.0.1:${port}`], + { + env: { ...process.env, HOME: home, USERPROFILE: home, HYPERFRAMES_BROWSER_PATH: "" }, + stdio: ["ignore", "ignore", "pipe"], + }, + ); + cleanups.push(() => { + child.kill("SIGKILL"); + for (const socket of sockets) socket.destroy(); + server.close(); + rmSync(home, { recursive: true, force: true }); + }); + let stderr = ""; + child.stderr.on("data", (chunk) => (stderr += chunk)); + + const exit = await new Promise<{ code: number | null }>((resolve) => { + const giveUp = setTimeout(() => resolve({ code: null }), 20_000); + child.once("exit", (code) => { + clearTimeout(giveUp); + resolve({ code }); + }); + }); + + expect(exit.code).toBe(1); + expect(stderr).toContain(`The Chrome download from 127.0.0.1:${port} stalled`); +}, 30_000); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 9ee452a1a59..c9e814baecf 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -21,6 +21,9 @@ import { pickSharedShell, } from "@hyperframes/engine/chrome-host-ceiling"; import { getFreeDiskMb } from "../telemetry/system.js"; +import http from "node:http"; +import https from "node:https"; +import { syncBuiltinESMExports } from "node:module"; import { normalizeErrorMessage } from "../utils/errorMessage.js"; import { runRenderSetupWorker } from "../utils/cancellableProcess.js"; import { unzipArchive } from "./unzipArchive.js"; @@ -230,6 +233,7 @@ export interface BrowserResult { export interface EnsureBrowserOptions { onProgress?: (downloadedBytes: number, totalBytes: number) => void; downloadStallMs?: number; + downloadBaseUrl?: string; // Re-download the managed build even when cached; only its version dir is replaced. force?: boolean; // Always resolve to OUR pinned `CHROME_VERSION` build (cached, or freshly @@ -696,6 +700,32 @@ function wrapDownloadFailureWithBrowserPathHint( return new Error(message, { cause: cause instanceof Error ? cause : undefined }); } +// Every request install() makes goes through http(s).request; keep them so a stall can destroy them. +function trackHttpRequests(): { abortAll: (reason: Error) => void; restore: () => void } { + const live = new Set(); + const originals = { http: http.request, https: https.request }; + const wrap = (original: F) => + ((...args: Parameters) => { + const request = (original as (...a: unknown[]) => http.ClientRequest)(...args); + live.add(request); + request.once("close", () => live.delete(request)); + return request; + }) as F; + http.request = wrap(originals.http); + https.request = wrap(originals.https); + syncBuiltinESMExports(); + return { + abortAll: (reason) => { + for (const request of live) request.destroy(reason); + }, + restore: () => { + http.request = originals.http; + https.request = originals.https; + syncBuiltinESMExports(); + }, + }; +} + const DOWNLOAD_STALL_MS = 60_000; const DOWNLOAD_ATTEMPTS = 3; @@ -703,24 +733,22 @@ class BrowserDownloadStalledError extends Error {} type ProgressCallback = (downloadedBytes: number, totalBytes: number) => void; -// install() takes no abort signal: a stalled transfer is abandoned, so the command and its lock stop waiting on it. +// install() takes no abort signal, so `abort` must close the sockets it opened or they keep the process alive. function withDownloadStallTimeout( start: (onProgress: ProgressCallback) => Promise, - { stallMs, host }: { stallMs: number; host: string }, + { stallMs, host, abort }: { stallMs: number; host: string; abort: (reason: Error) => void }, ): Promise { return new Promise((resolve, reject) => { let timer: NodeJS.Timeout | undefined; const arm = () => { clearTimeout(timer); - timer = setTimeout( - () => - reject( - new BrowserDownloadStalledError( - `The Chrome download from ${host} stalled: no data for ${stallMs / 1000}s.`, - ), - ), - stallMs, - ); + timer = setTimeout(() => { + const stalled = new BrowserDownloadStalledError( + `The Chrome download from ${host} stalled: no data for ${stallMs / 1000}s.`, + ); + abort(stalled); + reject(stalled); + }, stallMs); timer.unref?.(); }; arm(); @@ -775,8 +803,14 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise { const debug = debuglog(prefix); const isUnzip = prefix === "puppeteer:browsers:fileUtil"; @@ -798,15 +833,25 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise(call: (onProgress: ProgressCallback) => Promise) => - withDownloadStallTimeout( - (onProgress) => - call((downloaded, total) => { - options?.onProgress?.(downloaded, total); - onProgress(downloaded, total); - }), - { stallMs: options?.downloadStallMs ?? DOWNLOAD_STALL_MS, host: downloadHost }, - ); + const guarded = async (call: (onProgress: ProgressCallback) => Promise) => { + const requests = trackHttpRequests(); + try { + return await withDownloadStallTimeout( + (onProgress) => + call((downloaded, total) => { + options?.onProgress?.(downloaded, total); + onProgress(downloaded, total); + }), + { + stallMs: options?.downloadStallMs ?? DOWNLOAD_STALL_MS, + host: downloadHost, + abort: requests.abortAll, + }, + ); + } finally { + requests.restore(); + } + }; const unzipMayAskOnInput = process.platform !== "win32"; if (unzipMayAskOnInput) { const archive = await guarded((cb) => From f58d3470ace4cc9890134b9bf06ccfeb72d772ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 24 Oct 2026 12:08:52 -0400 Subject: [PATCH 3/4] fix(cli): the Chrome download runs in its own process and is killed when it stalls --- .../src/browser/downloadStall.process.test.ts | 48 ++++- packages/cli/src/browser/installInChild.ts | 86 ++++++++ packages/cli/src/browser/installProtocol.ts | 32 +++ packages/cli/src/browser/installRun.ts | 35 ++++ packages/cli/src/browser/manager.test.ts | 102 ++------- packages/cli/src/browser/manager.ts | 194 ++++++------------ packages/cli/src/renderSetupWorker.ts | 2 + packages/cli/src/utils/cancellableProcess.ts | 4 +- 8 files changed, 280 insertions(+), 223 deletions(-) create mode 100644 packages/cli/src/browser/installInChild.ts create mode 100644 packages/cli/src/browser/installProtocol.ts create mode 100644 packages/cli/src/browser/installRun.ts diff --git a/packages/cli/src/browser/downloadStall.process.test.ts b/packages/cli/src/browser/downloadStall.process.test.ts index f985bef5549..898f69688fd 100644 --- a/packages/cli/src/browser/downloadStall.process.test.ts +++ b/packages/cli/src/browser/downloadStall.process.test.ts @@ -1,5 +1,5 @@ import { spawn } from "node:child_process"; -import { mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; import { createServer, type Socket } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -13,9 +13,12 @@ afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup(); }); -it("exits with code 1 when the download host accepts the connection and never answers", async () => { +async function runFixture(onConnection: (socket: Socket) => void) { const sockets: Socket[] = []; - const server = createServer((socket) => sockets.push(socket)); + const server = createServer((socket) => { + sockets.push(socket); + onConnection(socket); + }); await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); const { port } = server.address() as { port: number }; const home = mkdtempSync(join(tmpdir(), "hf-download-stall-")); @@ -35,15 +38,40 @@ it("exits with code 1 when the download host accepts the connection and never an }); let stderr = ""; child.stderr.on("data", (chunk) => (stderr += chunk)); - - const exit = await new Promise<{ code: number | null }>((resolve) => { - const giveUp = setTimeout(() => resolve({ code: null }), 20_000); - child.once("exit", (code) => { + const startedAt = Date.now(); + const code = await new Promise((resolve) => { + const giveUp = setTimeout(() => resolve(null), 20_000); + child.once("exit", (exitCode) => { clearTimeout(giveUp); - resolve({ code }); + resolve(exitCode); + }); + }); + return { code, stderr, port, home, elapsedMs: Date.now() - startedAt }; +} + +it("exits with code 1 when the download host accepts the connection and never answers", async () => { + const run = await runFixture(() => {}); + + expect(run.code).toBe(1); + expect(run.stderr).toContain(`The Chrome download from 127.0.0.1:${run.port} stalled`); + expect(run.stderr).toContain("Retrying (2/2)"); + expect(existsSync(join(run.home, ".cache", "hyperframes", ".chrome.install.lock"))).toBe(false); +}, 30_000); + +it("does not call a download that keeps sending bytes a stall", async () => { + const run = await runFixture((socket) => { + socket.on("error", () => {}); + socket.once("data", () => { + socket.write("HTTP/1.1 200 OK\r\ncontent-length: 100000\r\n\r\n"); + const trickle = setInterval(() => socket.write("x"), 80); + setTimeout(() => { + clearInterval(trickle); + socket.destroy(); + }, 1_200); }); }); - expect(exit.code).toBe(1); - expect(stderr).toContain(`The Chrome download from 127.0.0.1:${port} stalled`); + expect(run.code).toBe(1); + expect(run.stderr).not.toContain("no data for"); + expect(run.elapsedMs).toBeGreaterThan(1_000); }, 30_000); diff --git a/packages/cli/src/browser/installInChild.ts b/packages/cli/src/browser/installInChild.ts new file mode 100644 index 00000000000..7c7bf27392b --- /dev/null +++ b/packages/cli/src/browser/installInChild.ts @@ -0,0 +1,86 @@ +import { runRenderSetupWorker } from "../utils/cancellableProcess.js"; +import { parseInstallLine } from "./installProtocol.js"; + +type InstallOptions = import("@puppeteer/browsers").InstallOptions; + +export type ChildInstallInput = Pick< + InstallOptions, + "cacheDir" | "browser" | "buildId" | "platform" | "baseUrl" +> & { unpack?: false }; + +interface InstalledPaths { + path: string; + executablePath: string; +} + +interface InstallHooks { + stallMs: number; + host: string; + onProgress?: (downloadedBytes: number, totalBytes: number) => void; + onLog: (line: string) => void; +} + +const CHILD_STARTUP_MS = 15_000; + +export class BrowserDownloadStalledError extends Error {} + +function lineReader(onLine: (line: string) => void): (chunk: string) => void { + let pending = ""; + return (chunk) => { + const lines = (pending + chunk).split("\n"); + pending = lines.pop() ?? ""; + for (const line of lines) onLine(line); + }; +} + +/** + * install() takes no abort signal and keeps its sockets open, so it runs in its own process: a stall + * kills that process and the OS closes the sockets, with nothing in this one left to clean up. + */ +export async function installInChild( + input: ChildInstallInput & { unpack: false }, + hooks: InstallHooks, +): Promise; +export async function installInChild( + input: ChildInstallInput, + hooks: InstallHooks, +): Promise; +export async function installInChild( + input: ChildInstallInput, + hooks: InstallHooks, +): Promise { + const abort = new AbortController(); + let timer: NodeJS.Timeout | undefined; + const arm = (waitMs: number) => { + clearTimeout(timer); + timer = setTimeout( + () => + abort.abort( + new BrowserDownloadStalledError( + `The Chrome download from ${hooks.host} stalled: no data for ${hooks.stallMs / 1000}s.`, + ), + ), + waitMs, + ); + timer.unref(); + }; + const onStdout = lineReader((line) => { + const event = parseInstallLine(line); + if (event?.kind === "log") hooks.onLog(event.text); + if (event?.kind !== "progress") return; + if (event.total > 0) hooks.onProgress?.(event.downloaded, event.total); + if (event.total > 0 && event.downloaded >= event.total) clearTimeout(timer); + else arm(hooks.stallMs); + }); + arm(CHILD_STARTUP_MS + hooks.stallMs); + try { + return await runRenderSetupWorker("browser-install", input, { + signal: abort.signal, + detached: false, + maxBufferBytes: 4 * 1024 * 1024, + onStdout, + }); + } finally { + clearTimeout(timer); + } +} diff --git a/packages/cli/src/browser/installProtocol.ts b/packages/cli/src/browser/installProtocol.ts new file mode 100644 index 00000000000..476bc06b7dd --- /dev/null +++ b/packages/cli/src/browser/installProtocol.ts @@ -0,0 +1,32 @@ +const PROGRESS_PREFIX = "HYPERFRAMES_BROWSER_INSTALL_PROGRESS:"; +const LOG_PREFIX = "HYPERFRAMES_BROWSER_INSTALL_LOG:"; + +export type InstallEvent = + | { kind: "progress"; downloaded: number; total: number } + | { kind: "log"; text: string }; + +export function installProgressLine(downloaded: number, total: number): string { + return `${PROGRESS_PREFIX}${JSON.stringify([downloaded, total])}\n`; +} + +export function installLogLine(text: string): string { + return `${LOG_PREFIX}${JSON.stringify(text)}\n`; +} + +export function parseInstallLine(line: string): InstallEvent | null { + try { + if (line.startsWith(PROGRESS_PREFIX)) { + const [downloaded, total] = JSON.parse(line.slice(PROGRESS_PREFIX.length)) as [ + number, + number, + ]; + return { kind: "progress", downloaded, total }; + } + if (line.startsWith(LOG_PREFIX)) { + return { kind: "log", text: JSON.parse(line.slice(LOG_PREFIX.length)) as string }; + } + } catch { + // a torn line is dropped: the next progress line carries the same information + } + return null; +} diff --git a/packages/cli/src/browser/installRun.ts b/packages/cli/src/browser/installRun.ts new file mode 100644 index 00000000000..3bc7bd00006 --- /dev/null +++ b/packages/cli/src/browser/installRun.ts @@ -0,0 +1,35 @@ +import { debuglog } from "node:util"; +import { installLogLine, installProgressLine } from "./installProtocol.js"; +import type { ChildInstallInput } from "./installInChild.js"; + +const PROGRESS_EVERY_MS = 100; + +/** Runs in the install child: reports progress and unzip log lines on stdout, returns what install() produced. */ +export async function runBrowserInstall(input: ChildInstallInput): Promise { + const { install } = await import("@puppeteer/browsers"); + let lastReport = 0; + process.stdout.write(installProgressLine(0, 0)); + const { unpack, ...rest } = input; + const options = { + ...rest, + downloadProgressCallback: (downloaded: number, total: number) => { + const now = Date.now(); + if (now - lastReport < PROGRESS_EVERY_MS && downloaded < total) return; + lastReport = now; + process.stdout.write(installProgressLine(downloaded, total)); + }, + logger: (prefix: string) => { + const debug = debuglog(prefix); + const isUnzip = prefix === "puppeteer:browsers:fileUtil"; + if (!isUnzip && !debug.enabled) return undefined; + return (...args: unknown[]) => { + const line = args.join(" "); + if (isUnzip) process.stdout.write(installLogLine(line)); + if (debug.enabled) debug(line); + }; + }, + }; + if (unpack === false) return install({ ...options, unpack: false }); + const { path, executablePath } = await install(options); + return { path, executablePath }; +} diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 76e85691b48..346392bf6f4 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -30,8 +30,6 @@ import { rmSync, writeFileSync, } from "node:fs"; -import { request } from "node:http"; -import { createServer, type Socket } from "node:net"; import { tmpdir } from "node:os"; import { basename, dirname, join, relative, sep } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -230,7 +228,6 @@ function installPuppeteerBrowsersMock( options: InstallMockOptions, ) => Promise<{ executablePath: string; path?: string }>; unzip?: (archivePath: string, folderPath: string) => Promise; - downloadUrl?: string; } = {}, ) { const impl = @@ -246,6 +243,27 @@ function installPuppeteerBrowsersMock( mockedPaths?.add(path).add(executablePath); return { executablePath, path }; }; + vi.doMock("./installInChild.js", async () => { + const { install } = await import("@puppeteer/browsers"); + return { + BrowserDownloadStalledError: class extends Error {}, + installInChild: ( + input: object, + hooks: { + onProgress?: (downloaded: number, total: number) => void; + onLog: (line: string) => void; + }, + ) => + install({ + ...input, + downloadProgressCallback: hooks.onProgress, + logger: (prefix: string) => + prefix === "puppeteer:browsers:fileUtil" + ? (...args: unknown[]) => hooks.onLog(args.join(" ")) + : undefined, + } as never), + }; + }); vi.doMock("./unzipArchive.js", () => ({ unzipArchive: opts.unzip ?? vi.fn(async () => true) })); vi.doMock("@puppeteer/browsers", () => ({ Browser: { CHROMEHEADLESSSHELL: "chrome-headless-shell" }, @@ -265,7 +283,6 @@ function installPuppeteerBrowsersMock( })), ), install: vi.fn().mockImplementation(stagedInstall), - ...(opts.downloadUrl && { getDownloadUrl: () => opts.downloadUrl }), })); } @@ -1437,83 +1454,6 @@ describe("downloadBrowser — install failure surfaces HYPERFRAMES_BROWSER_PATH ); }); -describe("downloadBrowser — a download that stops sending", () => { - const sockets: Socket[] = []; - - beforeEach(() => { - vi.resetModules(); - delete process.env["HYPERFRAMES_BROWSER_PATH"]; - delete process.env["PRODUCER_HEADLESS_SHELL_PATH"]; - installChildProcessMocks(); - }); - - afterEach(() => { - for (const socket of sockets.splice(0)) socket.destroy(); - vi.restoreAllMocks(); - vi.doUnmock("node:fs"); - vi.doUnmock("node:os"); - vi.doUnmock("node:child_process"); - vi.doUnmock("@puppeteer/browsers"); - }); - - it("gives up with the host named, retries, releases the install lock and points at HYPERFRAMES_BROWSER_PATH", async () => { - const server = createServer((socket) => sockets.push(socket)); - await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); - const { port } = server.address() as { port: number }; - const host = `127.0.0.1:${port}`; - installFsMocks({ existing: new Set([CACHE_ROOT]) }); - let attempts = 0; - installPuppeteerBrowsersMock({ - installedInHfCache: [], - downloadUrl: `http://${host}/chrome-headless-shell.zip`, - installImpl: () => { - attempts += 1; - return new Promise((_resolve, reject) => { - request({ host: "127.0.0.1", port, path: "/chrome-headless-shell.zip" }) - .on("error", reject) - .end(); - }); - }, - }); - const { ensureBrowser } = await import("./manager.js"); - - try { - const error = await ensureBrowser({ downloadStallMs: 100 }).catch((e: unknown) => e); - - expect(error).toBeInstanceOf(Error); - const message = (error as Error).message; - expect(message).toContain(`The Chrome download from ${host} stalled: no data for 0.1s`); - expect(message).toContain("HYPERFRAMES_BROWSER_PATH"); - expect(attempts).toBe(3); - expect(mockedPaths?.has(HF_LOCK)).toBe(false); - } finally { - for (const socket of sockets) socket.destroy(); - await new Promise((closed) => server.close(closed)); - } - }); - - it("does not time out a download that keeps making progress", async () => { - installFsMocks({ existing: new Set([CACHE_ROOT]) }); - installPuppeteerBrowsersMock({ - installedInHfCache: [], - installImpl: async (options) => { - const progress = (options as { downloadProgressCallback?: (d: number, t: number) => void }) - .downloadProgressCallback; - for (let bytes = 1; bytes <= 5; bytes++) { - await new Promise((tick) => setTimeout(tick, 60)); - progress?.(bytes, 5); - } - return { executablePath: HF_BINARY }; - }, - }); - const { ensureBrowser } = await import("./manager.js"); - - await expect(ensureBrowser({ downloadStallMs: 150 })).resolves.toMatchObject({ - source: "download", - }); - }); -}); - describe("downloadBrowser — too little disk for the browser", () => { beforeEach(() => { vi.resetModules(); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index c9e814baecf..1097717fd6c 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -13,7 +13,6 @@ import { writeFileSync, } from "node:fs"; import { homedir } from "node:os"; -import { debuglog } from "node:util"; import { basename, dirname, join, relative } from "node:path"; import { browserPathFromEnv, @@ -21,15 +20,12 @@ import { pickSharedShell, } from "@hyperframes/engine/chrome-host-ceiling"; import { getFreeDiskMb } from "../telemetry/system.js"; -import http from "node:http"; -import https from "node:https"; -import { syncBuiltinESMExports } from "node:module"; +import { BrowserDownloadStalledError, installInChild } from "./installInChild.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 { @@ -700,67 +696,9 @@ function wrapDownloadFailureWithBrowserPathHint( return new Error(message, { cause: cause instanceof Error ? cause : undefined }); } -// Every request install() makes goes through http(s).request; keep them so a stall can destroy them. -function trackHttpRequests(): { abortAll: (reason: Error) => void; restore: () => void } { - const live = new Set(); - const originals = { http: http.request, https: https.request }; - const wrap = (original: F) => - ((...args: Parameters) => { - const request = (original as (...a: unknown[]) => http.ClientRequest)(...args); - live.add(request); - request.once("close", () => live.delete(request)); - return request; - }) as F; - http.request = wrap(originals.http); - https.request = wrap(originals.https); - syncBuiltinESMExports(); - return { - abortAll: (reason) => { - for (const request of live) request.destroy(reason); - }, - restore: () => { - http.request = originals.http; - https.request = originals.https; - syncBuiltinESMExports(); - }, - }; -} - const DOWNLOAD_STALL_MS = 60_000; const DOWNLOAD_ATTEMPTS = 3; -class BrowserDownloadStalledError extends Error {} - -type ProgressCallback = (downloadedBytes: number, totalBytes: number) => void; - -// install() takes no abort signal, so `abort` must close the sockets it opened or they keep the process alive. -function withDownloadStallTimeout( - start: (onProgress: ProgressCallback) => Promise, - { stallMs, host, abort }: { stallMs: number; host: string; abort: (reason: Error) => void }, -): Promise { - return new Promise((resolve, reject) => { - let timer: NodeJS.Timeout | undefined; - const arm = () => { - clearTimeout(timer); - timer = setTimeout(() => { - const stalled = new BrowserDownloadStalledError( - `The Chrome download from ${host} stalled: no data for ${stallMs / 1000}s.`, - ); - abort(stalled); - reject(stalled); - }, stallMs); - timer.unref?.(); - }; - arm(); - start((downloaded, total) => { - if (total > 0 && downloaded >= total) clearTimeout(timer); - else arm(); - }) - .then(resolve, reject) - .finally(() => clearTimeout(timer)); - }); -} - async function retryOnStall(run: () => Promise, clear: () => void): Promise { for (let attempt = 1; ; attempt++) { try { @@ -773,12 +711,27 @@ async function retryOnStall(run: () => Promise, clear: () => void): Promis } } -async function downloadBrowser(options?: EnsureBrowserOptions): Promise { - if (isLinuxArm()) { - return ensureLinuxArmBrowser(options); +function downloadHostOf( + browsers: PuppeteerBrowsers, + platform: Parameters[1], + buildId: string, + baseUrl: string | undefined, +): string { + try { + const url = browsers.getDownloadUrl( + browsers.Browser.CHROMEHEADLESSSHELL, + platform, + buildId, + baseUrl, + ); + return new URL(url).host; + } catch { + return "the Chrome download host"; } +} - // Callers hold the install lock, so every leftover belongs to an install that was killed mid-way. +// Callers hold the install lock, so every leftover belongs to an install that was killed mid-way. +function assertRoomToDownload(): void { removeInstallLeftovers(); const freeMb = getFreeDiskMb(CACHE_ROOT_DIR); if (freeMb !== null && freeMb < ARCHIVE_PLUS_UNPACKED_SHELL_MB) { @@ -787,81 +740,60 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise>; - // Same filesystem as CACHE_DIR so the final rename is atomic. - const stagingDir = join(CACHE_ROOT_DIR, `${STAGING_PREFIX}${randomUUID()}`); - const clearStaging = () => 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[] = []; +function stagedInstaller( + browsers: PuppeteerBrowsers, + platform: BrowserPlatform, + stagingDir: string, + options: EnsureBrowserOptions | undefined, + unzipLog: { lines: string[] }, +) { const buildId = managedChromeVersion(); - let downloadHost = "the Chrome download host"; - try { - downloadHost = new URL( - browsers.getDownloadUrl( - Browser.CHROMEHEADLESSSHELL, - platform, - buildId, - options?.downloadBaseUrl, - ), - ).host; - } catch { - // keep the generic name: the host only makes the stall message more specific - } - const runInstall = async () => { - unzipLog = []; - const installOptions: InstallOptions & { unpack?: true } = { + const hooks = { + stallMs: options?.downloadStallMs ?? DOWNLOAD_STALL_MS, + host: downloadHostOf(browsers, platform, buildId, options?.downloadBaseUrl), + onProgress: options?.onProgress, + onLog: (line: string) => unzipLog.lines.push(line), + }; + return async () => { + unzipLog.lines = []; + const input = { cacheDir: stagingDir, - browser: Browser.CHROMEHEADLESSSHELL, + browser: browsers.Browser.CHROMEHEADLESSSHELL, buildId, platform, baseUrl: options?.downloadBaseUrl, - logger: (prefix) => { - const debug = debuglog(prefix); - const isUnzip = prefix === "puppeteer:browsers:fileUtil"; - if (!isUnzip && !debug.enabled) return undefined; - return (...args) => { - const line = args.join(" "); - if (isUnzip) unzipLog.push(line); - if (debug.enabled) debug(line); - }; - }, - }; - const guarded = async (call: (onProgress: ProgressCallback) => Promise) => { - const requests = trackHttpRequests(); - try { - return await withDownloadStallTimeout( - (onProgress) => - call((downloaded, total) => { - options?.onProgress?.(downloaded, total); - onProgress(downloaded, total); - }), - { - stallMs: options?.downloadStallMs ?? DOWNLOAD_STALL_MS, - host: downloadHost, - abort: requests.abortAll, - }, - ); - } finally { - requests.restore(); - } }; const unzipMayAskOnInput = process.platform !== "win32"; if (unzipMayAskOnInput) { - const archive = await guarded((cb) => - install({ ...installOptions, downloadProgressCallback: cb, unpack: false }), - ); - const into = new Cache(stagingDir).installationDir(installOptions.browser, platform, buildId); + const archive = await installInChild({ ...input, unpack: false }, hooks); + const into = new browsers.Cache(stagingDir).installationDir(input.browser, platform, buildId); await unzipArchive(archive, into); } - return guarded((cb) => install({ ...installOptions, downloadProgressCallback: cb })); + return installInChild(input, hooks); }; +} + +async function downloadBrowser(options?: EnsureBrowserOptions): Promise { + if (isLinuxArm()) { + return ensureLinuxArmBrowser(options); + } + assertRoomToDownload(); + const browsers = await loadPuppeteerBrowsers(); + const platform = browsers.detectBrowserPlatform(); + if (!platform) { + throw new Error(`Unsupported platform: ${process.platform} ${process.arch}`); + } + + // Same filesystem as CACHE_DIR so the final rename is atomic. + const stagingDir = join(CACHE_ROOT_DIR, `${STAGING_PREFIX}${randomUUID()}`); + const clearStaging = () => rmSync(stagingDir, { recursive: true, force: true }); + // install() only debug-logs each failed unzip tool before trying the next one; keep those lines. + const unzipLog = { lines: [] as string[] }; + const runInstall = stagedInstaller(browsers, platform, stagingDir, options, unzipLog); try { const staged = await retryOnStall( @@ -874,10 +806,10 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise { if (mode === "browser") return ensureBrowser(input); + if (mode === "browser-install") return runBrowserInstall(input); if (mode === "lint") { // Nobody waits on a background lint the way they wait on a Studio boot, so it takes idle CPU only. lowerPriority(); diff --git a/packages/cli/src/utils/cancellableProcess.ts b/packages/cli/src/utils/cancellableProcess.ts index 515eb01a147..59985b8a63d 100644 --- a/packages/cli/src/utils/cancellableProcess.ts +++ b/packages/cli/src/utils/cancellableProcess.ts @@ -16,6 +16,7 @@ export interface CancellableProcessOptions { env?: NodeJS.ProcessEnv; detached?: boolean; onSpawn?: (pid: number) => void; + onStdout?: (chunk: string) => void; } class CancellableProcessError extends Error { @@ -112,6 +113,7 @@ export function runCancellableProcess( child.stdout?.on("data", (chunk: Buffer | string) => { stdout += chunk.toString(); + options.onStdout?.(chunk.toString()); if (Buffer.byteLength(stdout) + Buffer.byteLength(stderr) > maxBufferBytes) { terminate(new Error(`Process output exceeded ${maxBufferBytes} bytes`), "ENOBUFS"); } @@ -194,7 +196,7 @@ function tsxLoaderFromCliInstall(): string { } export async function runRenderSetupWorker( - mode: "browser" | "lint" | "orphan-cleanup", + mode: "browser" | "browser-install" | "lint" | "orphan-cleanup", input: unknown, options: CancellableProcessOptions, ): Promise { From 67152eae561922834fcb563b8558411aa830a7b8 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 24 Oct 2026 12:31:24 -0400 Subject: [PATCH 4/4] test(cli): ignore the expected socket reset in the download stall test on Windows --- packages/cli/src/browser/downloadStall.process.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/browser/downloadStall.process.test.ts b/packages/cli/src/browser/downloadStall.process.test.ts index 898f69688fd..6f0af07b11f 100644 --- a/packages/cli/src/browser/downloadStall.process.test.ts +++ b/packages/cli/src/browser/downloadStall.process.test.ts @@ -17,6 +17,8 @@ async function runFixture(onConnection: (socket: Socket) => void) { const sockets: Socket[] = []; const server = createServer((socket) => { sockets.push(socket); + // Windows resets the held socket when the child is killed; that is expected. + socket.on("error", () => {}); onConnection(socket); }); await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); @@ -60,7 +62,6 @@ it("exits with code 1 when the download host accepts the connection and never an it("does not call a download that keeps sending bytes a stall", async () => { const run = await runFixture((socket) => { - socket.on("error", () => {}); socket.once("data", () => { socket.write("HTTP/1.1 200 OK\r\ncontent-length: 100000\r\n\r\n"); const trickle = setInterval(() => socket.write("x"), 80);