fix(cli): a Chrome download that stops sending is abandoned, retried, and releases the lock (#6222)

* fix(cli): a Chrome download that stops sending is abandoned, retried, and releases the lock

* fix(cli): a stalled Chrome download is aborted at the socket so the process can exit

* fix(cli): the Chrome download runs in its own process and is killed when it stalls

* test(cli): ignore the expected socket reset in the download stall test on Windows

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;
+}

packages/cli/src/browser/downloadStall.process.test.ts

@@ -0,0 +1,78 @@
+import { spawn } from "node:child_process";
+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";
+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();
+});
+
+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<void>((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 startedAt = Date.now();
+  const code = await new Promise<number | null>((resolve) => {
+    const giveUp = setTimeout(() => resolve(null), 20_000);
+    child.once("exit", (exitCode) => {
+      clearTimeout(giveUp);
+      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.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(run.code).toBe(1);
+  expect(run.stderr).not.toContain("no data for");
+  expect(run.elapsedMs).toBeGreaterThan(1_000);
+}, 30_000);

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<string>;
+export async function installInChild(
+  input: ChildInstallInput,
+  hooks: InstallHooks,
+): Promise<InstalledPaths>;
+export async function installInChild(
+  input: ChildInstallInput,
+  hooks: InstallHooks,
+): Promise<string | InstalledPaths> {
+  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<string | InstalledPaths>("browser-install", input, {
+      signal: abort.signal,
+      detached: false,
+      maxBufferBytes: 4 * 1024 * 1024,
+      onStdout,
+    });
+  } finally {
+    clearTimeout(timer);
+  }
+}

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;
+}

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<unknown> {
+  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 };
+}

packages/cli/src/browser/manager.test.ts

@@ -243,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" },

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,12 +20,12 @@ import {
   pickSharedShell,
 } from "@hyperframes/engine/chrome-host-ceiling";
 import { getFreeDiskMb } from "../telemetry/system.js";
+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<PuppeteerBrowsers> {
   try {
@@ -229,6 +228,8 @@ 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
@@ -695,12 +696,42 @@ function wrapDownloadFailureWithBrowserPathHint(
   return new Error(message, { cause: cause instanceof Error ? cause : undefined });
 }
 
-async function downloadBrowser(options?: EnsureBrowserOptions): Promise<BrowserResult> {
-  if (isLinuxArm()) {
-    return ensureLinuxArmBrowser(options);
-  }
+const DOWNLOAD_STALL_MS = 60_000;
+const DOWNLOAD_ATTEMPTS = 3;
 
-  // Callers hold the install lock, so every leftover belongs to an install that was killed mid-way.
+async function retryOnStall<T>(run: () => Promise<T>, clear: () => void): Promise<T> {
+  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();
+    }
+  }
+}
+
+function downloadHostOf(
+  browsers: PuppeteerBrowsers,
+  platform: Parameters<PuppeteerBrowsers["getDownloadUrl"]>[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.
+function assertRoomToDownload(): void {
   removeInstallLeftovers();
   const freeMb = getFreeDiskMb(CACHE_ROOT_DIR);
   if (freeMb !== null && freeMb < ARCHIVE_PLUS_UNPACKED_SHELL_MB) {
@@ -709,9 +740,50 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise<BrowserR
         `${freeMb} MB free at ${CACHE_ROOT_DIR}. Free disk space, then retry.`,
     );
   }
-  const { Browser, Cache, detectBrowserPlatform, install } = await loadPuppeteerBrowsers();
+}
 
-  const platform = detectBrowserPlatform();
+type BrowserPlatform = NonNullable<ReturnType<PuppeteerBrowsers["detectBrowserPlatform"]>>;
+
+function stagedInstaller(
+  browsers: PuppeteerBrowsers,
+  platform: BrowserPlatform,
+  stagingDir: string,
+  options: EnsureBrowserOptions | undefined,
+  unzipLog: { lines: string[] },
+) {
+  const buildId = managedChromeVersion();
+  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: browsers.Browser.CHROMEHEADLESSSHELL,
+      buildId,
+      platform,
+      baseUrl: options?.downloadBaseUrl,
+    };
+    const unzipMayAskOnInput = process.platform !== "win32";
+    if (unzipMayAskOnInput) {
+      const archive = await installInChild({ ...input, unpack: false }, hooks);
+      const into = new browsers.Cache(stagingDir).installationDir(input.browser, platform, buildId);
+      await unzipArchive(archive, into);
+    }
+    return installInChild(input, hooks);
+  };
+}
+
+async function downloadBrowser(options?: EnsureBrowserOptions): Promise<BrowserResult> {
+  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}`);
   }
@@ -720,47 +792,24 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise<BrowserR
   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[] = [];
-  const buildId = managedChromeVersion();
-  const runInstall = async () => {
-    unzipLog = [];
-    const installOptions: InstallOptions & { unpack?: true } = {
-      cacheDir: stagingDir,
-      browser: Browser.CHROMEHEADLESSSHELL,
-      buildId,
-      platform,
-      downloadProgressCallback: options?.onProgress,
-      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 unzipMayAskOnInput = process.platform !== "win32";
-    if (unzipMayAskOnInput) {
-      const archive = await install({ ...installOptions, unpack: false });
-      const into = new Cache(stagingDir).installationDir(installOptions.browser, platform, buildId);
-      await unzipArchive(archive, into);
-    }
-    return install(installOptions);
-  };
+  const unzipLog = { lines: [] as string[] };
+  const runInstall = stagedInstaller(browsers, platform, stagingDir, options, unzipLog);
 
   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 = [];
+    unzipLog.lines = [];
     return { executablePath: moveStagedInstallIntoCache(stagingDir, staged), source: "download" };
   } catch (err) {
-    throw wrapDownloadFailureWithBrowserPathHint(err, unzipLog);
+    throw wrapDownloadFailureWithBrowserPathHint(err, unzipLog.lines);
   } finally {
     clearStaging();
   }

packages/cli/src/renderSetupWorker.ts

@@ -1,4 +1,5 @@
 import { constants, setPriority } from "node:os";
+import { runBrowserInstall } from "./browser/installRun.js";
 import { ensureBrowser, releaseOwnedBrowserInstallLock } from "./browser/manager.js";
 import { lintProject } from "./utils/lintProject.js";
 import { killOrphanedProcesses } from "./utils/orphanCleanup.js";
@@ -28,6 +29,7 @@ function lowerPriority(): void {
 
 async function runMode(): Promise<unknown> {
   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();

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<T>(
-  mode: "browser" | "lint" | "orphan-cleanup",
+  mode: "browser" | "browser-install" | "lint" | "orphan-cleanup",
   input: unknown,
   options: CancellableProcessOptions,
 ): Promise<T> {