Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/**"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 6 additions & 4 deletions packages/cli/src/capture/captureCompositionFrame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/capture/captureTimeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}),
]);
Expand Down
60 changes: 60 additions & 0 deletions packages/cli/src/capture/navigateForCapture.browser.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
84 changes: 82 additions & 2 deletions packages/cli/src/capture/navigateForCapture.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
});
});
61 changes: 60 additions & 1 deletion packages/cli/src/capture/navigateForCapture.ts
Original file line number Diff line number Diff line change
@@ -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 };

Expand Down Expand Up @@ -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);
});
}
});
}
4 changes: 4 additions & 0 deletions packages/cli/src/commands/deprecationTestHarness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
}
Expand Down
11 changes: 3 additions & 8 deletions packages/cli/src/commands/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/commands/motionShot.browserClose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/motionShot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/commands/validate.browserClose.test.ts
Original file line number Diff line number Diff line change
@@ -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),
Expand All @@ -8,6 +12,7 @@ const browser = vi.hoisted(() => ({
on: () => undefined,
goto: async () => undefined,
evaluate: async () => undefined,
createCDPSession: fakeCdpSession,
})),
}));

Expand Down
Loading