From f69c4a0e3aa8feb1110b52d538f8ca127481c95d Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 4 Aug 2026 13:14:23 -0700 Subject: [PATCH] fix(engine): distinguish probe failure from a genuinely absent GPU A probe that could not run is no evidence about the GPU, so pointing the operator at GPU passthrough hid broken Chrome installs behind a phantom problem. Carry a cause off the probe and emit the matching remediation. Also un-exports buildUnverifiedHardwareGpuWarning (Fallow: engine test files are not audit entry points, so a test-only import would not have counted as a consumer) and covers the non-linux branch via the spy. --- .../src/services/browserManager.test.ts | 75 ++++++++++++++----- .../engine/src/services/browserManager.ts | 75 ++++++++++++++----- 2 files changed, 114 insertions(+), 36 deletions(-) diff --git a/packages/engine/src/services/browserManager.test.ts b/packages/engine/src/services/browserManager.test.ts index c1b0f3d1c..dfa068e5d 100644 --- a/packages/engine/src/services/browserManager.test.ts +++ b/packages/engine/src/services/browserManager.test.ts @@ -275,6 +275,34 @@ describe("resolveBrowserGpuMode", () => { expect(warn).not.toHaveBeenCalled(); }); + it("gives non-linux hosts the generic remediation, not the Docker one", async () => { + setMockWebGlProbe({ + hasWebGL: true, + vendor: "Google Inc. (Google)", + renderer: "ANGLE (Google, Vulkan 1.3.0 (SwiftShader Device))", + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(await resolveBrowserGpuMode("hardware", { platform: "darwin" })).toBe("hardware"); + const warning = String(warn.mock.calls[0]?.[0]); + expect(warning).toContain("host exposes a GPU"); + expect(warning).not.toContain("--gpus all"); + }); + + it("does not blame the GPU when the probe itself failed to launch", async () => { + // A probe that could not run is NO evidence about the GPU. Sending this + // operator to `--gpus all` would hide a broken Chrome install behind a + // phantom passthrough problem. + _setPuppeteerForTests({ + launch: vi.fn().mockRejectedValue(new Error("spawn ENOENT /bad/chrome")), + } as unknown as PuppeteerNode); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(await resolveBrowserGpuMode("hardware", { platform: "linux" })).toBe("hardware"); + const warning = String(warn.mock.calls[0]?.[0]); + expect(warning).toContain("GPU probe could not run"); + expect(warning).toContain("hyperframes doctor"); + expect(warning).not.toContain("--gpus all"); + }); + it("falls back to 'software' when the probe browser cannot launch", async () => { // No chromePath, env unset, and (in the test env) no system Chrome to find // → puppeteer.launch will throw → caller catches → software fallback. @@ -306,27 +334,40 @@ describe("resolveBrowserGpuMode", () => { expect(third).toBe("hardware"); }); - it("deduplicates concurrent auto-mode probes by caching the in-flight Promise", async () => { + it("deduplicates concurrent probes so only one Chrome launches", async () => { // Parallel coordinator fires N workers via Promise.all — without Promise- // level caching, a `--workers 4` render against a no-GPU host would launch - // 4 simultaneous probe Chromes. Verify all concurrent callers get the - // exact same Promise reference (proving the probe runs once, not N times). - const p1 = resolveBrowserGpuMode("auto", { - chromePath: "/definitely/not/a/real/chrome/binary", - browserTimeout: 2000, + // 4 simultaneous probe Chromes. Assert the launch count directly rather + // than Promise identity: `"auto"` and `"hardware"` now each adapt the + // shared cached Promise via `.then`, so identity is no longer the + // invariant — "the probe browser starts exactly once" is. + const { launch } = setMockWebGlProbe({ + hasWebGL: true, + vendor: "Google Inc. (Google)", + renderer: "ANGLE (Google, Vulkan 1.3.0 (SwiftShader Device))", }); - const p2 = resolveBrowserGpuMode("auto", { - chromePath: "/definitely/not/a/real/chrome/binary", - browserTimeout: 2000, - }); - const p3 = resolveBrowserGpuMode("auto", { - chromePath: "/definitely/not/a/real/chrome/binary", - browserTimeout: 2000, - }); - expect(p1).toBe(p2); - expect(p2).toBe(p3); - const results = await Promise.all([p1, p2, p3]); + const results = await Promise.all([ + resolveBrowserGpuMode("auto", { browserTimeout: 2000 }), + resolveBrowserGpuMode("auto", { browserTimeout: 2000 }), + resolveBrowserGpuMode("auto", { browserTimeout: 2000 }), + ]); expect(results).toEqual(["software", "software", "software"]); + expect(launch).toHaveBeenCalledTimes(1); + }); + + it("shares the one probe across mixed 'auto' and 'hardware' callers", async () => { + const { launch } = setMockWebGlProbe({ + hasWebGL: true, + vendor: "NVIDIA", + renderer: "NVIDIA GeForce RTX 3070", + }); + const results = await Promise.all([ + resolveBrowserGpuMode("auto"), + resolveBrowserGpuMode("hardware"), + resolveBrowserGpuMode("auto"), + ]); + expect(results).toEqual(["hardware", "hardware", "hardware"]); + expect(launch).toHaveBeenCalledTimes(1); }); it.each([ diff --git a/packages/engine/src/services/browserManager.ts b/packages/engine/src/services/browserManager.ts index c4b3c966b..899c3498b 100644 --- a/packages/engine/src/services/browserManager.ts +++ b/packages/engine/src/services/browserManager.ts @@ -422,7 +422,25 @@ export const _probeBeginFrameSupportForTests = probeBeginFrameSupport; export const _closeBrowserAfterFailedProbeForTests = closeBrowserAfterFailedProbe; /** - * Cached *in-flight or resolved* probe Promise for `resolveBrowserGpuMode("auto", ...)`. + * Outcome of the one-shot WebGL probe. + * + * `cause` distinguishes the two ways a probe lands on `"software"`, because + * they need OPPOSITE remediation: + * - `"no-gpu"` — the probe ran and Chrome reported a software renderer + * (SwiftShader / llvmpipe). Remediation: GPU passthrough. + * - `"probe-error"` — the probe itself failed (Chrome couldn't launch, bad + * executable path, sandbox denied). We have NO evidence + * about the GPU either way; telling the operator to fix GPU + * passthrough would send them chasing the wrong problem. + */ +interface GpuProbeOutcome { + mode: "software" | "hardware"; + cause?: "no-gpu" | "probe-error"; +} + +/** + * Cached *in-flight or resolved* probe Promise, shared by BOTH the `"auto"` + * and explicit `"hardware"` entry points of `resolveBrowserGpuMode`. * * Caching the Promise (rather than the resolved value) deduplicates concurrent * callers — the parallel coordinator runs N workers via `Promise.all`, so a @@ -430,10 +448,8 @@ export const _closeBrowserAfterFailedProbeForTests = closeBrowserAfterFailedProb * simultaneous probe Chromes. The first call assigns the Promise and every * other concurrent caller awaits the same one, paying the ~240 ms probe cost * exactly once per process lifetime. - * - * Exported for tests; production callers go through `resolveBrowserGpuMode`. */ -let _autoBrowserGpuModeCache: Promise<"software" | "hardware"> | undefined; +let _autoBrowserGpuModeCache: Promise | undefined; /** Test-only: reset the cached probe result. */ export function _resetAutoBrowserGpuModeCacheForTests(): void { @@ -479,7 +495,7 @@ async function probeAutoBrowserGpuMode(options: { chromePath?: string; browserTimeout?: number; platform?: NodeJS.Platform; -}): Promise<"software" | "hardware"> { +}): Promise { const platform = options.platform ?? process.platform; const browserTimeout = options.browserTimeout ?? DEFAULT_CONFIG.browserTimeout; const executablePath = options.chromePath ?? resolveHeadlessShellPath({}); @@ -487,7 +503,7 @@ async function probeAutoBrowserGpuMode(options: { if (ppt === null) { logResolvedBrowserGpuMode("software", "puppeteer unavailable"); - return "software"; + return { mode: "software", cause: "probe-error" }; } try { @@ -498,10 +514,10 @@ async function probeAutoBrowserGpuMode(options: { }); const resolved = resolveWebGlProbeMode(info); logResolvedBrowserGpuMode(resolved, describeWebGlProbe(info)); - return resolved; + return resolved === "hardware" ? { mode: "hardware" } : { mode: "software", cause: "no-gpu" }; } catch (err) { logResolvedBrowserGpuMode("software", formatProbeFailure(err)); - return "software"; + return { mode: "software", cause: "probe-error" }; } } @@ -542,32 +558,53 @@ export function resolveBrowserGpuMode( if (mode === "software") return Promise.resolve(mode); _autoBrowserGpuModeCache ??= probeAutoBrowserGpuMode(options); - if (mode === "auto") return _autoBrowserGpuModeCache; + if (mode === "auto") return _autoBrowserGpuModeCache.then((probed) => probed.mode); return _autoBrowserGpuModeCache.then((probed) => { - // Warn once per process, not once per caller: `createCaptureSession` + // Warn once per cache lifetime, not once per caller: `createCaptureSession` // resolves the mode for the probe browser AND every parallel worker, so // an un-deduplicated warning prints N+1 times and buries itself. - if (probed === "software" && !_unverifiedHardwareGpuWarned) { + if (probed.mode === "software" && !_unverifiedHardwareGpuWarned) { _unverifiedHardwareGpuWarned = true; - console.warn(buildUnverifiedHardwareGpuWarning(options.platform ?? process.platform)); + console.warn( + buildUnverifiedHardwareGpuWarning(options.platform ?? process.platform, probed.cause), + ); } return "hardware"; }); } -/** One-shot latch for the explicit-hardware-probed-to-software warning. */ +/** + * Latch for the explicit-hardware-probed-to-software warning: fires once per + * cache lifetime (re-armed by `_resetAutoBrowserGpuModeCacheForTests`). + */ let _unverifiedHardwareGpuWarned = false; /** - * Warning text for "you asked for hardware GPU, the probe found none". + * Warning text for "you asked for hardware GPU and we could not confirm it". * - * Names the observable symptom (the render still completes, just on CPU) and - * the platform's actual remediation, so the operator doesn't have to infer it - * from Chrome's `Automatic fallback to software WebGL` warning. Exported for - * tests. + * Splits on `cause` because the two failure shapes need opposite remediation. + * A probe that RAN and saw SwiftShader is a GPU-passthrough problem. A probe + * that could not run tells us nothing about the GPU — pointing that operator + * at `--gpus all` would send them chasing a phantom while their Chrome + * install is the actual fault. */ -export function buildUnverifiedHardwareGpuWarning(platform: NodeJS.Platform | string): string { +function buildUnverifiedHardwareGpuWarning( + platform: NodeJS.Platform | string, + cause: GpuProbeOutcome["cause"], +): string { + if (cause === "probe-error") { + return ( + "[hyperframes] browserGpuMode=hardware was requested, but the GPU probe could not run, " + + "so hardware acceleration is UNVERIFIED — if Chrome falls back to software WebGL the " + + "capture will run at CPU speed. Honouring the explicit request anyway.\n" + + " This is a probe failure, not evidence of a missing GPU: see the " + + "`browserGpuMode probe → software (probe failed ...)` line above for the underlying " + + "error, which usually means Chrome could not launch (bad HYPERFRAMES_BROWSER_PATH, " + + "missing shared libraries, or a denied sandbox) rather than a GPU problem.\n" + + " Run `hyperframes doctor` to check the Chrome install." + ); + } const remediation = platform === "linux" ? "Inside Docker, the container needs GPU passthrough: `--gpus all` with the NVIDIA " +