fix(cli): page loads stop timing out while a busy machine's Chrome warms up (#5705)

* fix(cli): page loads stop timing out while a busy machine's Chrome warms up

* fix(cli): keep the page-load helper comment-free and its files in the bench filter

.github/workflows/ci.yml

@@ -249,7 +249,7 @@ jobs:
               - "packages/cli/src/utils/**"
               - "packages/cli/src/ui/**"
               - "packages/cli/src/{background-removal,registry,telemetry}/**"
-              - "packages/cli/src/capture/{captureCompositionFrame,readBoundedResponse}.ts"
+              - "packages/cli/src/capture/{captureCompositionFrame,captureTimeout,navigateForCapture,readBoundedResponse}.ts"
               - "packages/cli/src/media-use/lib/config-lock.mjs"
               - "packages/cli/scripts/**"
               - "packages/core/stubs/**"

packages/cli/src/capture/captureCompositionFrame.browserFallback.test.ts

@@ -57,6 +57,11 @@ function fakeBrowser() {
     evaluateOnNewDocument: vi.fn(async () => undefined),
     setViewport: vi.fn(async () => undefined),
     goto: vi.fn(async () => undefined),
+    createCDPSession: async () => ({
+      send: async () => undefined,
+      on: () => undefined,
+      detach: async () => undefined,
+    }),
     waitForFunction: vi.fn(async () => undefined),
     evaluate: vi.fn(async () => undefined),
     setRequestInterception: vi.fn(async (_on: boolean) => undefined),

packages/cli/src/capture/captureCompositionFrame.ts

@@ -18,6 +18,7 @@ import { windowsChromeCrashRemediation } from "../browser/windowsCrash.js";
 import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport.js";
 import { normalizeErrorMessage } from "../utils/errorMessage.js";
 import { resolveDiagnosticNavigationTimeoutMs } from "../utils/renderArgs.js";
+import { gotoWhenSent } from "./navigateForCapture.js";
 
 const SHADER_TRANSITIONS_TIMEOUT_MS = 90_000;
 const CAPTURE_SETTLE_MS = 1500;
@@ -265,10 +266,11 @@ export async function openSettledCompositionPage(
     await page.setViewport(viewport);
     if (answer) await answerPageInProcess(page, url, answer);
     await options.beforeNavigate?.(page);
-    await page.goto(url, {
-      waitUntil: "domcontentloaded",
-      timeout: resolveDiagnosticNavigationTimeoutMs(process.env, options.navigationTimeoutMs),
-    });
+    await gotoWhenSent(
+      page,
+      url,
+      resolveDiagnosticNavigationTimeoutMs(process.env, options.navigationTimeoutMs),
+    );
     await assertWebGpuAdapterAvailable(page, requiresWebGpu, softwareWebGpu);
     const renderReadyTimedOut = !(await waitForCompositionSettle(page, options));
     return { browser: chromeBrowser, page, renderReadyTimedOut };

packages/cli/src/capture/captureTimeout.ts

@@ -58,9 +58,10 @@ export async function withRemainingBudget<T>(
   work: Promise<T>,
   remainingMs: number,
   label: string,
+  timeoutError: () => Error = () => new StageBudgetTimeoutError(label, Math.max(0, remainingMs)),
 ): Promise<T> {
   if (!(remainingMs > 0)) {
-    throw new StageBudgetTimeoutError(label, Math.max(0, remainingMs));
+    throw timeoutError();
   }
 
   let timer: ReturnType<typeof setTimeout> | undefined;
@@ -69,7 +70,7 @@ export async function withRemainingBudget<T>(
       work,
       new Promise<never>((_resolve, reject) => {
         timer = setTimeout(() => {
-          reject(new StageBudgetTimeoutError(label, remainingMs));
+          reject(timeoutError());
         }, remainingMs);
       }),
     ]);

packages/cli/src/capture/navigateForCapture.browser.test.ts

@@ -0,0 +1,60 @@
+import { once } from "node:events";
+import { createServer, type ServerResponse } from "node:http";
+import type { AddressInfo } from "node:net";
+import puppeteer, { type Browser } from "puppeteer-core";
+import { afterAll, beforeAll, expect, it } from "vitest";
+import { ensureBrowser } from "../browser/manager.js";
+import { documentRequestSent } from "./navigateForCapture.js";
+
+let browser: Browser;
+let held: ServerResponse | undefined;
+let requested: () => void = () => undefined;
+const server = createServer((_req, res) => {
+  held = res;
+  requested();
+});
+
+beforeAll(async () => {
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
+  const { executablePath } = await ensureBrowser();
+  browser = await puppeteer.launch({ executablePath, headless: true, args: ["--no-sandbox"] });
+}, 60_000);
+
+afterAll(async () => {
+  const closed = once(server.close(), "close");
+  server.closeAllConnections();
+  await Promise.all([browser?.close(), closed]);
+});
+
+it("settles when Chrome sends the document request, before the server answers", async () => {
+  const page = await browser.newPage();
+  const cdp = await page.createCDPSession();
+  await cdp.send("Network.enable");
+  const serverGotRequest = new Promise<void>((resolve) => (requested = resolve));
+  const sent = documentRequestSent(cdp);
+  const navigation = page.goto(`http://127.0.0.1:${(server.address() as AddressInfo).port}/`, {
+    waitUntil: "domcontentloaded",
+    timeout: 0,
+  });
+  await serverGotRequest;
+  await sent;
+  held?.end("<p>page</p>");
+  await navigation;
+  expect(await page.content()).toContain("page");
+  await page.close();
+});
+
+it("settles for a page answered by interception, which never reaches the network", async () => {
+  const page = await browser.newPage();
+  const cdp = await page.createCDPSession();
+  await cdp.send("Network.enable");
+  await page.setRequestInterception(true);
+  page.on("request", (request) =>
+    request.respond({ status: 200, contentType: "text/html", body: "<p>answered</p>" }),
+  );
+  const sent = documentRequestSent(cdp);
+  await page.goto("http://in-process.invalid/", { waitUntil: "domcontentloaded", timeout: 0 });
+  await sent;
+  expect(await page.content()).toContain("answered");
+  await page.close();
+});

packages/cli/src/capture/navigateForCapture.test.ts

@@ -1,6 +1,8 @@
-import { describe, expect, it, vi } from "vitest";
-import { TimeoutError } from "puppeteer-core";
+import { EventEmitter } from "node:events";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { TimeoutError, type Page } from "puppeteer-core";
 import {
+  gotoWhenSent,
   isNavigationTimeoutError,
   navigateForCapture,
   networkIdleAttemptTimeoutMs,
@@ -117,3 +119,81 @@ describe("navigateForCapture", () => {
     });
   });
 });
+
+/** A page whose goto keeps Puppeteer's contract: a non-zero timeout rejects the navigation. */
+function fakeChromePage() {
+  const session = Object.assign(new EventEmitter(), {
+    send: async () => undefined,
+    detach: vi.fn(async () => undefined),
+  });
+  let load: (response: unknown) => void = () => undefined;
+  const page = {
+    createCDPSession: async () => session,
+    goto: (_url: string, { timeout }: { timeout: number }) =>
+      new Promise((resolve, reject) => {
+        load = resolve;
+        if (timeout > 0) {
+          setTimeout(
+            () => reject(new TimeoutError(`Navigation timeout of ${timeout} ms exceeded`)),
+            timeout,
+          );
+        }
+      }),
+  } as unknown as Page;
+  return {
+    page,
+    session,
+    navigate: () =>
+      session.emit("Network.requestWillBeSent", { requestId: "doc", type: "Document" }),
+    send: () => session.emit("Network.requestWillBeSentExtraInfo", { requestId: "doc" }),
+    load: (response: unknown) => load(response),
+  };
+}
+
+describe("gotoWhenSent", () => {
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it("gives the page its whole budget after a cold Chrome is slow to send the request", async () => {
+    vi.useFakeTimers();
+    const chrome = fakeChromePage();
+    const response = { status: () => 200 };
+    const opened = gotoWhenSent(chrome.page, "http://127.0.0.1:1/", 10_000);
+    await vi.advanceTimersByTimeAsync(0);
+    chrome.navigate();
+    await vi.advanceTimersByTimeAsync(25_000);
+    chrome.send();
+    await vi.advanceTimersByTimeAsync(9_000);
+    chrome.load(response);
+    await expect(opened).resolves.toBe(response);
+    expect(chrome.session.detach).toHaveBeenCalledOnce();
+  });
+
+  it("fails as a navigation timeout when Chrome never sends the request", async () => {
+    vi.useFakeTimers();
+    const chrome = fakeChromePage();
+    const opened = gotoWhenSent(chrome.page, "http://127.0.0.1:1/", 10_000);
+    const failed = expect(opened).rejects.toSatisfy(
+      (error: unknown) =>
+        isNavigationTimeoutError(error) &&
+        String(error).includes("did not send the page request within 30000 ms"),
+    );
+    await vi.advanceTimersByTimeAsync(0);
+    chrome.navigate();
+    await vi.advanceTimersByTimeAsync(30_000);
+    await failed;
+  });
+
+  it("fails when the page does not load within its budget after the request is sent", async () => {
+    vi.useFakeTimers();
+    const chrome = fakeChromePage();
+    const opened = gotoWhenSent(chrome.page, "http://127.0.0.1:1/", 10_000);
+    const failed = expect(opened).rejects.toThrow("Navigation timeout of 10000 ms exceeded");
+    await vi.advanceTimersByTimeAsync(0);
+    chrome.send();
+    chrome.navigate();
+    await vi.advanceTimersByTimeAsync(10_000);
+    await failed;
+  });
+});

packages/cli/src/capture/navigateForCapture.ts

@@ -1,4 +1,5 @@
-import { isNavigationTimeoutError } from "./captureTimeout.js";
+import { TimeoutError, type CDPSession, type HTTPResponse, type Page } from "puppeteer-core";
+import { isNavigationTimeoutError, withRemainingBudget } from "./captureTimeout.js";
 
 export { isNavigationTimeoutError };
 
@@ -61,3 +62,61 @@ export async function navigateForCapture<TResponse>(
     fellBackFromNetworkIdle: true,
   };
 }
+
+const PUPPETEER_LAUNCH_DEFAULT_TIMEOUT_MS = 30_000;
+const SENT_OR_ANSWERED_WITHOUT_NETWORK = [
+  "Network.requestWillBeSentExtraInfo",
+  "Network.responseReceived",
+] as const;
+
+export async function gotoWhenSent(
+  page: Page,
+  url: string,
+  timeoutMs: number,
+  coldChromeFirstRequestTimeoutMs = PUPPETEER_LAUNCH_DEFAULT_TIMEOUT_MS,
+): Promise<HTTPResponse | null> {
+  const cdp = await page.createCDPSession();
+  try {
+    await cdp.send("Network.enable");
+    const sent = documentRequestSent(cdp);
+    const navigation = page.goto(url, { waitUntil: "domcontentloaded", timeout: 0 });
+    const navigationAwaitedBelow = navigation.catch(() => undefined);
+    await withRemainingBudget(
+      Promise.race([sent, navigationAwaitedBelow]),
+      coldChromeFirstRequestTimeoutMs,
+      "sending the page request",
+      () =>
+        new TimeoutError(
+          `Navigation timeout: Chrome did not send the page request within ${coldChromeFirstRequestTimeoutMs} ms`,
+        ),
+    );
+    return await withRemainingBudget(
+      navigation,
+      timeoutMs,
+      "loading the page",
+      () => new TimeoutError(`Navigation timeout of ${timeoutMs} ms exceeded`),
+    );
+  } finally {
+    await cdp.detach().catch(() => undefined);
+  }
+}
+
+export function documentRequestSent(cdp: CDPSession): Promise<void> {
+  const documents = new Set<string>();
+  const sent = new Set<string>();
+  return new Promise((resolve) => {
+    const settle = (requestId: string) => {
+      if (documents.has(requestId) && sent.has(requestId)) resolve();
+    };
+    cdp.on("Network.requestWillBeSent", ({ requestId, type }) => {
+      if (type === "Document") documents.add(requestId);
+      settle(requestId);
+    });
+    for (const event of SENT_OR_ANSWERED_WITHOUT_NETWORK) {
+      cdp.on(event, ({ requestId }) => {
+        sent.add(requestId);
+        settle(requestId);
+      });
+    }
+  });
+}

packages/cli/src/commands/deprecationTestHarness.ts

@@ -19,6 +19,10 @@ const FAKE_PROJECT = {
   indexPath: "/fake-project/index.html",
 };
 
+export async function fakeCdpSession() {
+  return { send: async () => undefined, on: () => undefined, detach: async () => undefined };
+}
+
 export function resolveProjectMock() {
   return { resolveProject: vi.fn(() => FAKE_PROJECT) };
 }

packages/cli/src/commands/layout.ts

@@ -8,6 +8,7 @@ import type { Example } from "./_examples.js";
 import { c } from "../ui/colors.js";
 import { resolveProject } from "../utils/project.js";
 import { resolveDiagnosticNavigationTimeoutMs } from "../utils/renderArgs.js";
+import { gotoWhenSent } from "../capture/navigateForCapture.js";
 import { normalizeErrorMessage } from "../utils/errorMessage.js";
 import { serveStaticProjectHtml } from "../utils/staticProjectServer.js";
 import { printDeprecationNotice, withMeta } from "../utils/updateCheck.js";
@@ -184,10 +185,7 @@ async function alignViewportToComposition(
   });
 
   await page.setViewport(size);
-  await page.goto(url, {
-    waitUntil: "domcontentloaded",
-    timeout: resolveDiagnosticNavigationTimeoutMs(),
-  });
+  await gotoWhenSent(page, url, resolveDiagnosticNavigationTimeoutMs());
 }
 
 async function runLayoutAudit(
@@ -237,10 +235,7 @@ async function runLayoutAudit(
     const page = await chromeBrowser.newPage();
     await installPageFunctionGuard(page);
     await page.setViewport({ width: 1920, height: 1080 });
-    await page.goto(server.url, {
-      waitUntil: "domcontentloaded",
-      timeout: resolveDiagnosticNavigationTimeoutMs(),
-    });
+    await gotoWhenSent(page, server.url, resolveDiagnosticNavigationTimeoutMs());
     await assertWebGpuAdapterAvailable(page, requiresWebGpu, softwareWebGpu);
     await alignViewportToComposition(page, server.url);
     await waitForRuntimeReady(page, opts.timeout);

packages/cli/src/commands/motionShot.browserClose.test.ts

@@ -2,10 +2,12 @@ import { mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, expect, it, vi } from "vitest";
+import { fakeCdpSession } from "./deprecationTestHarness.js";
 
 const browser = vi.hoisted(() => ({
   close: vi.fn(async () => undefined),
   newPage: vi.fn(async () => ({
+    createCDPSession: fakeCdpSession,
     setViewport: async () => undefined,
     goto: async () => undefined,
     evaluate: async () => undefined,

packages/cli/src/commands/motionShot.ts

@@ -14,6 +14,7 @@ import { launchManagedBrowser, resolveManagedGpuMode } from "../browser/launch.j
 import { mkdirSync, statSync, writeFileSync } from "node:fs";
 import { dirname, resolve } from "node:path";
 import { resolveDiagnosticNavigationTimeoutMs } from "../utils/renderArgs.js";
+import { gotoWhenSent } from "../capture/navigateForCapture.js";
 import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport.js";
 import { waitForRuntimeReady } from "../capture/captureCompositionFrame.js";
 import {
@@ -425,9 +426,8 @@ async function openCompositionPage(
   });
   try {
     const page = await browser.newPage();
-    const navigationTimeout = resolveDiagnosticNavigationTimeoutMs();
     await page.setViewport(size);
-    await page.goto(url, { waitUntil: "domcontentloaded", timeout: navigationTimeout });
+    await gotoWhenSent(page, url, resolveDiagnosticNavigationTimeoutMs());
     await assertWebGpuAdapterAvailable(page, requiresWebGpu, softwareWebGpu);
     await waitForRuntimeReady(page, 10000);
     await page

packages/cli/src/commands/validate.browserClose.test.ts

@@ -1,5 +1,9 @@
 import { afterEach, expect, it, vi } from "vitest";
-import { resolveProjectMock, runAndCaptureStdio } from "./deprecationTestHarness.js";
+import {
+  fakeCdpSession,
+  resolveProjectMock,
+  runAndCaptureStdio,
+} from "./deprecationTestHarness.js";
 
 const browser = vi.hoisted(() => ({
   close: vi.fn(async () => undefined),
@@ -8,6 +12,7 @@ const browser = vi.hoisted(() => ({
     on: () => undefined,
     goto: async () => undefined,
     evaluate: async () => undefined,
+    createCDPSession: fakeCdpSession,
   })),
 }));