diff --git a/packages/cli/src/browser/downloadStall.fixture.ts b/packages/cli/src/browser/downloadStall.fixture.ts index 9d322a05e4..8a7369d4b6 100644 --- a/packages/cli/src/browser/downloadStall.fixture.ts +++ b/packages/cli/src/browser/downloadStall.fixture.ts @@ -1,12 +1,13 @@ // fallow-ignore-file unused-file import { ensureBrowser } from "./manager.js"; -const [baseUrl] = process.argv.slice(2); +const [baseUrl, attempts] = process.argv.slice(2); try { await ensureBrowser({ preferManagedChrome: true, downloadStallMs: 200, downloadBaseUrl: baseUrl, + ...(attempts ? { downloadAttempts: Number(attempts) } : {}), }); } catch (error) { process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); diff --git a/packages/cli/src/browser/downloadStall.process.test.ts b/packages/cli/src/browser/downloadStall.process.test.ts index 20cf3d525c..074581de04 100644 --- a/packages/cli/src/browser/downloadStall.process.test.ts +++ b/packages/cli/src/browser/downloadStall.process.test.ts @@ -13,7 +13,7 @@ afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup(); }); -async function runFixture(onConnection: (socket: Socket) => void) { +async function runFixture(onConnection: (socket: Socket) => void, attempts?: number) { const sockets: Socket[] = []; const server = createServer((socket) => { sockets.push(socket); @@ -26,7 +26,13 @@ async function runFixture(onConnection: (socket: Socket) => void) { const home = mkdtempSync(join(tmpdir(), "hf-download-stall-")); const child = spawn( process.execPath, - ["--import", "tsx", fixturePath, `http://127.0.0.1:${port}`], + [ + "--import", + "tsx", + fixturePath, + `http://127.0.0.1:${port}`, + ...(attempts ? [String(attempts)] : []), + ], { env: { ...process.env, HOME: home, USERPROFILE: home, HYPERFRAMES_BROWSER_PATH: "" }, stdio: ["ignore", "ignore", "pipe"], @@ -51,10 +57,20 @@ async function runFixture(onConnection: (socket: Socket) => void) { } it("exits with code 1 when the download host accepts the connection and never answers", async () => { - const run = await runFixture(() => {}); + const run = await runFixture(() => {}, 3); 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); }, 120_000); + +it("gives up after the first stall when not on a terminal, naming the env var and doctor", 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).not.toContain("Retrying"); + expect(run.stderr).toContain("HYPERFRAMES_BROWSER_PATH"); + expect(run.stderr).toContain("hyperframes doctor"); +}, 120_000); diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 346392bf6f..245610481a 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -1216,6 +1216,67 @@ describe("findBrowser — cache resolution", () => { expect(warnSpy).not.toHaveBeenCalled(); }); + it("ensureBrowser fails fast naming a configured browser path that does not exist", async () => { + installFsMocks({ existing: new Set(["/usr/bin/chromium"]) }); + installPuppeteerBrowsersMock(); + process.env["HYPERFRAMES_BROWSER_PATH"] = "/opt/typo/chrome"; + + const { ensureBrowser } = await import("./manager.js"); + + await expect(ensureBrowser()).rejects.toThrow( + "HYPERFRAMES_BROWSER_PATH is set to /opt/typo/chrome, which does not exist", + ); + }); + + it("retryOnStall gives up after one 75 s stall when attempts is 1, and retries to ~195 s when 3", async () => { + vi.useFakeTimers(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const { retryOnStall } = await import("./manager.js"); + const { BrowserDownloadStalledError } = await import("./installInChild.js"); + const stalled = () => + new Promise((_, reject) => + setTimeout(() => reject(new BrowserDownloadStalledError("stalled")), 75_000), + ); + const once = vi.fn(stalled); + const first = expect(retryOnStall(once, () => {}, 1)).rejects.toThrow("stalled"); + await vi.advanceTimersByTimeAsync(75_000); + await first; + expect(once).toHaveBeenCalledTimes(1); + + const thrice = vi.fn(stalled); + const all = expect(retryOnStall(thrice, () => {}, 3)).rejects.toThrow("stalled"); + await vi.advanceTimersByTimeAsync(3 * 75_000); + await all; + expect(thrice).toHaveBeenCalledTimes(3); + } finally { + warn.mockRestore(); + vi.useRealTimers(); + } + }); + + it("withBrowserWaitNotice speaks every 15 s while the task runs and stops when it ends", async () => { + vi.useFakeTimers(); + try { + const notify = vi.fn(); + let finish = () => {}; + const task = new Promise((resolve) => (finish = () => resolve("done"))); + const { withBrowserWaitNotice } = await import("./manager.js"); + const result = withBrowserWaitNotice(task, notify, 15_000); + await vi.advanceTimersByTimeAsync(14_000); + expect(notify).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(31_000); + expect(notify).toHaveBeenCalledTimes(3); + expect(notify.mock.calls[0]?.[0]).toContain("HYPERFRAMES_BROWSER_PATH"); + finish(); + await expect(result).resolves.toBe("done"); + await vi.advanceTimersByTimeAsync(60_000); + expect(notify).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + it("prefers HYPERFRAMES_BROWSER_PATH over PRODUCER_HEADLESS_SHELL_PATH when both are set", async () => { // Tiebreak matches `render.ts` — the CLI-native name canonicalizes; the // engine name is a compatibility alias. If both are set the caller almost diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 1097717fd6..7f21928792 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -229,6 +229,8 @@ export interface BrowserResult { export interface EnsureBrowserOptions { onProgress?: (downloadedBytes: number, totalBytes: number) => void; downloadStallMs?: number; + // Stalled-download attempts; default 3 on a terminal, 1 elsewhere (see ensureBrowser). + downloadAttempts?: number; downloadBaseUrl?: string; // Re-download the managed build even when cached; only its version dir is replaced. force?: boolean; @@ -542,15 +544,52 @@ async function ensureLinuxArmBrowser(options?: EnsureBrowserOptions): Promise
OUR pinned cache -> auto-download * (puppeteer-cache preference and system Chrome are both skipped). */ +const BROWSER_WAIT_NOTICE_MS = 15_000; + +/** Runs `task`, saying so every 15 s while it is still going, so a first-use download is never silent. */ +export async function withBrowserWaitNotice( + task: Promise, + notify: (message: string) => void = console.warn, + everyMs = BROWSER_WAIT_NOTICE_MS, +): Promise { + const timer = setInterval( + () => + notify( + "[hyperframes] Still getting Chrome ready; the first use downloads it. " + + "Set HYPERFRAMES_BROWSER_PATH to use an installed Chrome instead.", + ), + everyMs, + ); + timer.unref(); + try { + return await task; + } finally { + clearInterval(timer); + } +} + export async function ensureBrowser(options?: EnsureBrowserOptions): Promise { - if (options?.signal) return ensureBrowserInOwnedProcess(options); - return ensureBrowserInCurrentProcess(options); + // Tools that time out at 120 s lose to three stalled attempts (~4 min); one stall then a clear error wins. + const resolved = { + ...options, + downloadAttempts: options?.downloadAttempts ?? (process.stdout.isTTY ? DOWNLOAD_ATTEMPTS : 1), + }; + const task = resolved.signal + ? ensureBrowserInOwnedProcess(resolved) + : ensureBrowserInCurrentProcess(resolved); + return resolved.onProgress ? task : withBrowserWaitNotice(task); } // fallow-ignore-next-line complexity async function ensureBrowserInCurrentProcess( options?: EnsureBrowserOptions, ): Promise { + const configured = browserPathFromEnv(); + if (configured && !existsSync(configured.path)) { + throw new Error( + `${configured.name} is set to ${configured.path}, which does not exist. Fix the path or unset it.`, + ); + } const fromEnv = findFromEnv(); if (fromEnv) return fromEnv; @@ -600,6 +639,7 @@ async function ensureBrowserInOwnedProcess(options: EnsureBrowserOptions): Promi { force: options.force, preferManagedChrome: options.preferManagedChrome, + downloadAttempts: options.downloadAttempts, }, { signal, @@ -689,7 +729,7 @@ function wrapDownloadFailureWithBrowserPathHint( `Failed to install chrome-headless-shell ${managedChromeVersion()}: ${original}${unzipErrors}\n\n` + `Point hyperframes at an already-installed Chrome/Chromium instead:\n\n` + ` export HYPERFRAMES_BROWSER_PATH="${example}"\n\n` + - `Then re-run your command. Any Chrome build works for the screenshot ` + + `Run \`hyperframes doctor\` to see which Chrome was found. Then re-run your command. Any Chrome build works for the screenshot ` + `capture path; install a real chrome-headless-shell later if you need the ` + `perf-optimized BeginFrame path. Alternatively, run inside the hyperframes ` + `Docker image which ships a compatible headless-shell.`; @@ -699,13 +739,17 @@ function wrapDownloadFailureWithBrowserPathHint( const DOWNLOAD_STALL_MS = 60_000; const DOWNLOAD_ATTEMPTS = 3; -async function retryOnStall(run: () => Promise, clear: () => void): Promise { +export async function retryOnStall( + run: () => Promise, + clear: () => void, + attempts = DOWNLOAD_ATTEMPTS, +): 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}).`); + if (!(err instanceof BrowserDownloadStalledError) || attempt >= attempts) throw err; + console.warn(`[hyperframes] ${err.message} Retrying (${attempt}/${attempts - 1}).`); clear(); } } @@ -804,6 +848,7 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise