fix(producer): harden capture against timeouts, transient tab deaths, and OOM (#1842)

Four independent capture-infra hardening changes for the P2-5 failure bucket (~15K err / ~7K users):

- protocolTimeout auto-scales by device-scaled output area (applied before probe launch, since it's immutable post ppt.launch()).
- Single bounded transient retry (MAX_TRANSIENT_CAPTURE_RETRIES=1) on Target closed / Page crashed in the parallel disk-capture path; abort short-circuits before retry.
- Narrow OOM classification (Set maximum size exceeded etc., disjoint from transient) → actionable guidance naming output dims.
- StreamingEncoder.getExitError() threads FFmpeg's real exit reason into frame-0 encoder-death errors.

Render-reliability workstream P2-5. Success measured on PostHog dashboard 1783183.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
James Russo
2026-07-01 18:50:16 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 180f368af1
commit c0c3abf0f1
14 changed files with 584 additions and 14 deletions
+41 -1
View File
@@ -1,5 +1,5 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { resolveConfig, DEFAULT_CONFIG } from "./config.js";
import { resolveConfig, DEFAULT_CONFIG, scaleProtocolTimeoutForComposition } from "./config.js";
import { isLowMemorySystem } from "./services/systemMemory.js";
describe("resolveConfig", () => {
@@ -211,3 +211,43 @@ describe("resolveConfig", () => {
});
});
});
describe("scaleProtocolTimeoutForComposition", () => {
const base = 300_000;
it("keeps the base timeout for a reference-or-smaller canvas", () => {
// 1080p == reference area → factor 1, no scale.
expect(scaleProtocolTimeoutForComposition(base, { width: 1920, height: 1080 })).toBe(base);
// Smaller than reference → still the base (never scales down).
expect(scaleProtocolTimeoutForComposition(base, { width: 1280, height: 720 })).toBe(base);
});
it("scales up proportionally with output pixel area", () => {
// 4K == 4× the reference area, which stays under the 30-minute ceiling.
const scaled = scaleProtocolTimeoutForComposition(base, { width: 3840, height: 2160 });
expect(scaled).toBeGreaterThan(base);
expect(scaled).toBe(base * 4);
});
it("clamps at the 30-minute ceiling for a pathological canvas", () => {
// 8K == 16× area → 4.8M ms, clamped to the 30-minute ceiling.
const scaled = scaleProtocolTimeoutForComposition(base, { width: 7680, height: 4320 });
expect(scaled).toBe(1_800_000);
});
it("never lowers a base timeout that already exceeds the ceiling", () => {
// Base above the 30-min ceiling + a large canvas: must not clamp below base.
const highBase = 2_400_000;
expect(
scaleProtocolTimeoutForComposition(highBase, { width: 3840, height: 2160 }),
).toBeGreaterThanOrEqual(highBase);
});
it("returns the base timeout for degenerate dimensions", () => {
expect(scaleProtocolTimeoutForComposition(base, { width: 0, height: 1080 })).toBe(base);
expect(scaleProtocolTimeoutForComposition(base, { width: 1920, height: 0 })).toBe(base);
expect(scaleProtocolTimeoutForComposition(base, { width: Number.NaN, height: 1080 })).toBe(
base,
);
});
});
+47
View File
@@ -252,6 +252,53 @@ export const DEFAULT_CONFIG: EngineConfig = {
debug: false,
};
/**
* Reference canvas area for the baseline `protocolTimeout`: 1080p. A single CDP
* call (`Runtime.callFunctionOn` seek+paint, or `Page.captureScreenshot`)
* scales with the *output pixel area* it has to render/serialize — NOT with the
* frame count (that governs total wall-clock, capped separately by the ffmpeg
* streaming inactivity timeout). A fixed 300s ceiling intermittently kills
* legitimate slow-but-valid renders on large canvases with
* `Runtime.callFunctionOn timed out`, so we scale the per-call ceiling with
* area.
*/
const PROTOCOL_TIMEOUT_REFERENCE_PIXELS = 1920 * 1080;
/**
* Absolute ceiling on the scaled protocol timeout (30 minutes). Bounds the
* blast radius: a genuinely wedged CDP call must still eventually fail rather
* than hang for an unbounded time on a pathologically large composition.
*/
const MAX_SCALED_PROTOCOL_TIMEOUT_MS = 1_800_000;
/**
* Scale a base `protocolTimeout` up for oversized compositions.
*
* Scales by output pixel area (`width*height / reference`) — where width/height
* are the *device-scaled output* dimensions (the pixels a single CDP call
* actually renders/serializes), not the CSS composition size. Clamped to
* `[baseTimeout, max(baseTimeout, MAX_SCALED_PROTOCOL_TIMEOUT_MS)]`: never
* scales DOWN (a small composition — or a base already above the ceiling —
* keeps the configured base), and only ever raises. Pure function; exported
* for tests.
*/
export function scaleProtocolTimeoutForComposition(
baseTimeoutMs: number,
dims: { width: number; height: number },
): number {
const { width, height } = dims;
if (!Number.isFinite(width) || !Number.isFinite(height) || width <= 0 || height <= 0) {
return baseTimeoutMs;
}
const factor = (width * height) / PROTOCOL_TIMEOUT_REFERENCE_PIXELS;
if (factor <= 1) return baseTimeoutMs;
const scaled = Math.ceil(baseTimeoutMs * factor);
// Ceiling is `max(base, MAX)` so an explicit base above the ceiling is never
// lowered (preserves the "only ever raise" contract for all callers).
const ceiling = Math.max(baseTimeoutMs, MAX_SCALED_PROTOCOL_TIMEOUT_MS);
return Math.min(ceiling, Math.max(baseTimeoutMs, scaled));
}
function memoryAdaptiveCacheLimit(): number {
const total = getSystemTotalMb();
if (total < 4096) return 32;
+7 -1
View File
@@ -43,7 +43,12 @@ export type {
} from "./types.js";
// ── Configuration ──────────────────────────────────────────────────────────────
export { resolveConfig, DEFAULT_CONFIG, type EngineConfig } from "./config.js";
export {
resolveConfig,
DEFAULT_CONFIG,
scaleProtocolTimeoutForComposition,
type EngineConfig,
} from "./config.js";
export {
DEFAULT_VP9_CPU_USED,
MAX_VP9_CPU_USED,
@@ -83,6 +88,7 @@ export {
prepareCaptureSessionForReuse,
type CaptureSession,
isTransientBrowserError,
isMemoryExhaustionError,
type BeforeCaptureHook,
type DiscardWarmupInnerCapture,
} from "./services/frameCapture.js";
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { isTransientBrowserError } from "./frameCapture.js";
import { isMemoryExhaustionError, isTransientBrowserError } from "./frameCapture.js";
describe("isTransientBrowserError", () => {
it.each([
@@ -45,3 +45,47 @@ describe("isTransientBrowserError", () => {
expect(isTransientBrowserError(42)).toBe(false);
});
});
describe("isMemoryExhaustionError", () => {
it.each([
"Set maximum size exceeded",
"Map maximum size exceeded",
"Invalid array length",
"Invalid string length",
"Array buffer allocation failed",
"Cannot create a string longer than 0x1fffffe8 characters",
"FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory",
"JavaScript heap out of memory",
])("returns true for memory-exhaustion error: %s", (message) => {
expect(isMemoryExhaustionError(new Error(message))).toBe(true);
});
it.each([
"Target closed",
"Runtime.callFunctionOn timed out",
"net::ERR_NAME_NOT_RESOLVED",
"Composition duration is 0",
"",
// Deliberately NOT matched — a bare "out of memory" substring appears in
// benign WebGL/GPU console noise; only the specific V8/Node allocation
// signatures (and "JavaScript heap out of memory") count.
"WebGL: CONTEXT_LOST_WEBGL loseContext: context out of memory",
"GL_OUT_OF_MEMORY: out of memory",
])("returns false for non-memory error: %s", (message) => {
expect(isMemoryExhaustionError(new Error(message))).toBe(false);
});
it("handles non-Error values", () => {
expect(isMemoryExhaustionError("Set maximum size exceeded")).toBe(true);
expect(isMemoryExhaustionError("some other string")).toBe(false);
expect(isMemoryExhaustionError(null)).toBe(false);
expect(isMemoryExhaustionError(undefined)).toBe(false);
});
// A memory-exhaustion error is a resource ceiling, not a flaky-tab hiccup —
// it must NOT be classified as transient (a retry re-hits the same wall).
it("is disjoint from transient classification", () => {
expect(isTransientBrowserError(new Error("Set maximum size exceeded"))).toBe(false);
expect(isMemoryExhaustionError(new Error("Target closed"))).toBe(false);
});
});
@@ -2006,3 +2006,35 @@ export function isTransientBrowserError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error);
return TRANSIENT_BROWSER_ERROR_PATTERNS.some((pattern) => pattern.test(message));
}
// ── Memory-exhaustion classification ────────────────────────────────────────
// A render can run the Node process (or a page-side allocation) out of memory
// on an oversized composition — huge canvas, thousands of frames, or a very
// large frame cache. These surface as cryptic V8 RangeErrors ("Set maximum
// size exceeded", "Invalid array length"/"string length", "Array buffer
// allocation failed") or a hard V8 heap-limit abort. They are NOT transient
// (a retry re-hits the same ceiling) and NOT composition-logic bugs — they're
// resource limits. Classify them so the caller can surface actionable guidance
// (lower resolution / fps / duration, or enable low-memory mode) instead of a
// raw RangeError.
// Deliberately specific: each pattern is a distinct V8/Node allocation-failure
// signature. We intentionally do NOT match a bare /out of memory/ — that
// substring appears in benign browser-console noise (WebGL `CONTEXT_LOST … out
// of memory`, GPU driver notes) that gets carried into the error path, and
// misclassifying it would replace the real failure message with generic OOM
// guidance.
const MEMORY_EXHAUSTION_ERROR_PATTERNS = [
/Set maximum size exceeded/i,
/Map maximum size exceeded/i,
/Invalid (?:array|string) length/i,
/Array buffer allocation failed/i,
/Cannot create a string longer than/i,
/Reached heap limit/i,
/JavaScript heap out of memory/i,
];
export function isMemoryExhaustionError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error);
return MEMORY_EXHAUSTION_ERROR_PATTERNS.some((pattern) => pattern.test(message));
}
@@ -526,6 +526,55 @@ describe("spawnStreamingEncoder lifecycle and cleanup", () => {
expect(result.error).toContain("Encoder error");
});
it("getExitError surfaces the ffmpeg failure reason after a non-zero exit", async () => {
const { spawn, calls } = createSpawnSpy();
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { spawnStreamingEncoder } = await import("./streamingEncoder.js");
const dir = mkdtempSync(join(tmpdir(), "se-exiterr-"));
const encoder = await spawnStreamingEncoder(join(dir, "out.mp4"), baseOptions);
const proc = calls[0]!.proc;
// While running, there is no exit error to report.
expect(encoder.getExitError()).toBeUndefined();
proc.stderr.emit("data", Buffer.from("Unknown encoder 'libx264'\n"));
await new Promise<void>((resolve) => {
process.nextTick(() => {
proc.emit("close", 1);
resolve();
});
});
// After a non-zero exit, the reason is available synchronously — this is
// what `ensureFrameWritten` reads to turn "encoder exited before frame 0"
// into an actionable message.
const exitError = encoder.getExitError();
expect(exitError).toContain("FFmpeg exited with code 1");
expect(exitError).toContain("Unknown encoder 'libx264'");
});
it("getExitError returns undefined after a clean exit", async () => {
const { spawn, calls } = createSpawnSpy();
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { spawnStreamingEncoder } = await import("./streamingEncoder.js");
const dir = mkdtempSync(join(tmpdir(), "se-exitok-"));
const encoder = await spawnStreamingEncoder(join(dir, "out.mp4"), baseOptions);
const proc = calls[0]!.proc;
await new Promise<void>((resolve) => {
process.nextTick(() => {
proc.emit("close", 0);
resolve();
});
});
expect(encoder.getExitError()).toBeUndefined();
});
it("returns a failure result (does NOT throw) when ffmpeg fails to spawn (ENOENT)", async () => {
const { spawn, calls } = createSpawnSpy();
vi.resetModules();
@@ -141,6 +141,13 @@ export interface StreamingEncoder {
writeFrame: (buffer: Buffer) => Promise<boolean>;
close: () => Promise<StreamingEncoderResult>;
getExitStatus: () => "running" | "success" | "error";
/**
* The FFmpeg failure reason (exit code + tail of stderr), or `undefined`
* while the process is still running / exited cleanly. Lets a `writeFrame`
* that returned `false` because FFmpeg died surface WHY it died (bad args,
* unsupported codec, disk full) instead of a bare "encoder exited" message.
*/
getExitError: () => string | undefined;
}
/**
@@ -600,6 +607,11 @@ export async function spawnStreamingEncoder(
},
getExitStatus: () => exitStatus,
getExitError: () => {
if (exitStatus !== "error") return undefined;
return formatFfmpegError(exitCode, stderr);
},
};
return encoder;