From edd99398797051f8910e777bf5bf94aea5756f4a Mon Sep 17 00:00:00 2001 From: The one -Niu <912906590@qq.com> Date: Tue, 13 Oct 2026 22:35:45 +0800 Subject: [PATCH] fix(browser): give the DevToolsActivePort wait one deadline owner (#2265) * Single owner for the Chrome DevToolsActivePort startup deadline Cold Chrome startup tolerance was spelled three ways for one wait: the helper fell back to 15,000 ms, the runtime fixture lifecycle passed a literal 60,000, and the real-browser verifier kept a local 60,000 constant. A per-caller literal quietly changes how long a lane tolerates a cold browser before it fails, and the failure reported only elapsed_ms, so a reader could not see which ceiling was hit. Export BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS as the owner, use it for the helper fallback and both call sites, and report startup_timeout_ms in BrowserStartupError. A source guard keeps callers from restating the number. * Pin the DevToolsActivePort deadline guard to the startup call argument --------- Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- .../lib/browser-runtime-fixture-lifecycle.mjs | 7 ++- scripts/verify-real-browser.mjs | 8 ++- src/browser/BrowserProcessStartup.ts | 29 ++++++++-- .../browser/browserProcessStartup.test.ts | 55 +++++++++++++++++++ 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/scripts/lib/browser-runtime-fixture-lifecycle.mjs b/scripts/lib/browser-runtime-fixture-lifecycle.mjs index d39617f58..88a76e573 100644 --- a/scripts/lib/browser-runtime-fixture-lifecycle.mjs +++ b/scripts/lib/browser-runtime-fixture-lifecycle.mjs @@ -8,7 +8,10 @@ import { spawnOwnedProviderProcess } from "../../dist/process/ProviderProcess.js import { createSystemProcessOwnershipHost } from "../../dist/process/ProcessOwnershipObservation.js"; import { liveProcesses } from "../../dist/process/ProcessOwnershipProcessTree.js"; import { CdpConnection } from "../../dist/browser/CdpConnection.js"; -import { waitForBrowserDevtoolsPort } from "../../dist/browser/BrowserProcessStartup.js"; +import { + BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, + waitForBrowserDevtoolsPort, +} from "../../dist/browser/BrowserProcessStartup.js"; /** Launch the synthetic fixture browser and await CDP in its own verified process group. */ export async function startRuntimeFixtureBrowser(executable, origin) { @@ -50,7 +53,7 @@ export async function startRuntimeFixtureBrowser(executable, origin) { stderr: () => stderr, // Match the broader real-browser verifier's allowance for cold Chrome // startup on shared runners; early process failures still reject at once. - timeoutMs: 60_000, + timeoutMs: BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, }); return { port, diff --git a/scripts/verify-real-browser.mjs b/scripts/verify-real-browser.mjs index 48c9bec08..27c0c5e52 100644 --- a/scripts/verify-real-browser.mjs +++ b/scripts/verify-real-browser.mjs @@ -9,7 +9,10 @@ import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { CdpBrowserProvider } from "../dist/browser/CdpBrowserProvider.js"; -import { waitForBrowserDevtoolsPort } from "../dist/browser/BrowserProcessStartup.js"; +import { + BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, + waitForBrowserDevtoolsPort, +} from "../dist/browser/BrowserProcessStartup.js"; import { createBrowserScenarioProvider } from "../dist/composition/browserScenario.js"; import { inspectWebPageInputSchema, @@ -47,7 +50,6 @@ import { artifactCliEvidence, artifactMcpResult } from "./lib/artifact-e2e.mjs"; import { verifyScenarioEnvironment } from "./lib/browser-scenario-environment-e2e.mjs"; import { verifyScenarioStorage } from "./lib/browser-scenario-storage-e2e.mjs"; -const REAL_BROWSER_STARTUP_TIMEOUT_MS = 60_000; const SCENARIO_SECRET_VALUE = "rea-browser-verifier-secret"; const SCENARIO_URL_SECRET_VALUE = "rea-browser-url-verifier-secret"; process.env.REA_BROWSER_VERIFIER_SECRET = SCENARIO_SECRET_VALUE; @@ -92,7 +94,7 @@ try { executable, activePortPath: join(profile, "DevToolsActivePort"), stderr: () => stderr, - timeoutMs: REAL_BROWSER_STARTUP_TIMEOUT_MS, + timeoutMs: BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, }); const endpoint = `http://127.0.0.1:${String(port)}`; const provider = new CdpBrowserProvider(); diff --git a/src/browser/BrowserProcessStartup.ts b/src/browser/BrowserProcessStartup.ts index 85eae0553..48367c432 100644 --- a/src/browser/BrowserProcessStartup.ts +++ b/src/browser/BrowserProcessStartup.ts @@ -14,6 +14,7 @@ export class BrowserStartupError extends Error { readonly failure: BrowserStartupFailure; readonly executable: string; readonly elapsedMs: number; + readonly startupTimeoutMs: number; readonly exitCode: number | null; readonly signalCode: NodeJS.Signals | null; readonly stderr: string; @@ -23,6 +24,7 @@ export class BrowserStartupError extends Error { readonly failure: BrowserStartupFailure; readonly executable: string; readonly elapsedMs: number; + readonly startupTimeoutMs: number; readonly exitCode: number | null; readonly signalCode: NodeJS.Signals | null; readonly stderr: string; @@ -33,12 +35,22 @@ export class BrowserStartupError extends Error { this.failure = diagnostics.failure; this.executable = diagnostics.executable; this.elapsedMs = diagnostics.elapsedMs; + this.startupTimeoutMs = diagnostics.startupTimeoutMs; this.exitCode = diagnostics.exitCode; this.signalCode = diagnostics.signalCode; this.stderr = diagnostics.stderr; } } +/** + * Single owner of the wait for Chrome to publish its DevToolsActivePort. + * + * Callers that need a different budget pass `timeoutMs`; nobody else should + * restate this number, because a per-caller literal silently changes how long + * a cold Chrome launch is tolerated before the failure names its ceiling. + */ +export const BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS = 60_000; + /** Owned child process and startup policy for Chrome CDP discovery. */ export interface BrowserStartupOptions { readonly child: ChildProcess; @@ -53,23 +65,23 @@ export interface BrowserStartupOptions { export const waitForBrowserDevtoolsPort = async ( options: BrowserStartupOptions, ): Promise => { - const timeoutMs = options.timeoutMs ?? 15_000; + const timeoutMs = options.timeoutMs ?? BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS; const pollIntervalMs = options.pollIntervalMs ?? 25; const startedAt = performance.now(); - const terminal = childTermination(options, startedAt); + const terminal = childTermination(options, startedAt, timeoutMs); const deadline = startedAt + timeoutMs; while (true) { const terminalState = currentTerminalState(options.child); if (terminalState !== null) - throw startupError(options, startedAt, terminalState); + throw startupError(options, startedAt, timeoutMs, terminalState); const port = await readDevtoolsPort(options.activePortPath); if (port !== null) return port; const remainingMs = deadline - performance.now(); if (remainingMs <= 0) - throw startupError(options, startedAt, { failure: "timeout" }); + throw startupError(options, startedAt, timeoutMs, { failure: "timeout" }); const outcome = await Promise.race([ terminal, @@ -88,11 +100,12 @@ type TerminalState = const childTermination = ( options: BrowserStartupOptions, startedAt: number, + startupTimeoutMs: number, ): Promise => new Promise((resolve) => { options.child.once("error", (cause: unknown) => { resolve( - startupError(options, startedAt, { + startupError(options, startedAt, startupTimeoutMs, { failure: "spawn-error", cause, }), @@ -103,6 +116,7 @@ const childTermination = ( startupError( options, startedAt, + startupTimeoutMs, options.child.signalCode === null ? { failure: "exited" } : { failure: "signalled" }, @@ -120,6 +134,7 @@ const currentTerminalState = (child: ChildProcess): TerminalState | null => { const startupError = ( options: BrowserStartupOptions, startedAt: number, + startupTimeoutMs: number, state: TerminalState, ): BrowserStartupError => new BrowserStartupError( @@ -127,6 +142,7 @@ const startupError = ( failure: state.failure, executable: options.executable, elapsedMs: Math.max(0, Math.round(performance.now() - startedAt)), + startupTimeoutMs, exitCode: options.child.exitCode, signalCode: options.child.signalCode, stderr: options.stderr(), @@ -156,9 +172,10 @@ const browserStartupMessage = ( diagnostics: { readonly executable: string; readonly elapsedMs: number; + readonly startupTimeoutMs: number; readonly exitCode: number | null; readonly signalCode: NodeJS.Signals | null; readonly stderr: string; }, ): string => - `Chrome CDP startup ${failure}; executable=${diagnostics.executable}; elapsed_ms=${String(diagnostics.elapsedMs)}; exit_code=${diagnostics.exitCode === null ? "null" : String(diagnostics.exitCode)}; signal=${diagnostics.signalCode ?? "null"}; stderr=${diagnostics.stderr.length === 0 ? "" : diagnostics.stderr}`; + `Chrome CDP startup ${failure}; executable=${diagnostics.executable}; elapsed_ms=${String(diagnostics.elapsedMs)}; startup_timeout_ms=${String(diagnostics.startupTimeoutMs)}; exit_code=${diagnostics.exitCode === null ? "null" : String(diagnostics.exitCode)}; signal=${diagnostics.signalCode ?? "null"}; stderr=${diagnostics.stderr.length === 0 ? "" : diagnostics.stderr}`; diff --git a/tests/boundary/browser/browserProcessStartup.test.ts b/tests/boundary/browser/browserProcessStartup.test.ts index 91c4f58e9..b1cfaeafe 100644 --- a/tests/boundary/browser/browserProcessStartup.test.ts +++ b/tests/boundary/browser/browserProcessStartup.test.ts @@ -1,3 +1,4 @@ +import { readFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect } from "vitest"; @@ -9,6 +10,7 @@ import { } from "../../support/process/processFixture.js"; import { + BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, BrowserStartupError, waitForBrowserDevtoolsPort, } from "../../../src/browser/BrowserProcessStartup.js"; @@ -86,15 +88,68 @@ describe("browser process startup", () => { failure: "timeout", exitCode: null, signalCode: null, + startupTimeoutMs: 20, }); expect(String(failure)).toContain("stderr="); + expect(String(failure)).toContain("startup_timeout_ms=20"); } finally { child.kill("SIGKILL"); expect(await waitForExit(child, 5_000)).toBe(true); } }); + + it("names the owner's deadline when a caller omits one", async ({ + processes, + }) => { + const root = await temporaryRoot(); + const child = processes.spawn(process.execPath, [ + "-e", + "setTimeout(() => process.exit(3), 10)", + ]); + + const failure = await waitForBrowserDevtoolsPort({ + child, + executable: process.execPath, + activePortPath: join(root, "DevToolsActivePort"), + stderr: () => "owner default fixture", + pollIntervalMs: 5, + }).catch((cause: unknown) => cause); + + expect(failure).toBeInstanceOf(BrowserStartupError); + expect(failure).toMatchObject({ + failure: "exited", + startupTimeoutMs: BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS, + }); + expect(String(failure)).toContain( + `startup_timeout_ms=${String(BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS)}`, + ); + }); + + it("keeps one owner for the DevToolsActivePort deadline", async () => { + expect(BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS).toBe(60_000); + const callers = [ + "../../../scripts/verify-real-browser.mjs", + "../../../scripts/lib/browser-runtime-fixture-lifecycle.mjs", + ]; + for (const caller of callers) { + const source = await readFile(new URL(caller, import.meta.url), "utf8"); + expect(devtoolsPortDeadlines(source)).toEqual([ + ["BROWSER_DEVTOOLS_STARTUP_TIMEOUT_MS"], + ]); + } + }); }); const temporaryRoot = async (): Promise => { return createTestTempDirectory("rea-browser-startup-"); }; + +const devtoolsPortDeadlines = (source: string): string[][] => { + const calls = /waitForBrowserDevtoolsPort\(\{(?[\s\S]*?)\}\)/gu; + const deadlines = /\btimeoutMs:\s*(?[^\n,]+)/gu; + return [...source.matchAll(calls)].map((call) => + [...String(call.groups?.body).matchAll(deadlines)].map((deadline) => + String(deadline.groups?.value).trim(), + ), + ); +};