mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(producer): retry probe stage on transient browser errors (#1688)
* fix(producer): retry probe stage on transient browser errors (#1687) The distributed render plan stage crashes when headless Chrome encounters a transient frame detachment ("Navigating frame was detached") during browser probe, with no retry logic. The plan tarball is never uploaded, and all downstream chunk workers fail with S3 404. Add a retry-with-fresh-session mechanism to the probe stage: - `isTransientBrowserError()` classifier in the engine identifies 9 known transient Puppeteer/Chrome errors (frame detached, target closed, session closed, protocol error, page crashed, execution context destroyed, etc.). - `runProbeStage()` wraps browser session creation + initialization in a retry loop (max 2 attempts). On transient error: logs structured diagnostics (attempt, isTransient, error message, elapsed time), closes the crashed session cleanly, creates a fresh browser, and retries. Non- transient errors throw immediately without consuming retry budget. - 17 unit tests for the error classifier, 3 integration tests for retry behavior (successful retry, immediate throw on non-transient, exhaust retry budget on persistent transient). Closes #1687 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback — widen retry scope, deduplicate patterns - Move createCaptureSession inside the retry try/catch so browser launch failures (Failed to launch the browser process, ECONNREFUSED) are also retried — not just initializeSession errors. - Deduplicate transient error patterns: remove "Protocol error.*Target closed" (subsumed by "Target closed") and "Navigation failed because browser has disconnected" (subsumed by "browser has disconnected"). - Add browser launch failure patterns: "Failed to launch the browser process" and "ECONNREFUSED". - Add test for createCaptureSession transient throw (browser launch retry). - Update test mock comment to document sync requirement with engine pattern list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
899b8faa12
commit
546b2d770b
@@ -82,6 +82,7 @@ export {
|
||||
getCapturePerfSummary,
|
||||
prepareCaptureSessionForReuse,
|
||||
type CaptureSession,
|
||||
isTransientBrowserError,
|
||||
type BeforeCaptureHook,
|
||||
type DiscardWarmupInnerCapture,
|
||||
} from "./services/frameCapture.js";
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isTransientBrowserError } from "./frameCapture.js";
|
||||
|
||||
describe("isTransientBrowserError", () => {
|
||||
it.each([
|
||||
"Navigating frame was detached",
|
||||
"Target closed",
|
||||
"Session closed. Most likely the page has been closed.",
|
||||
"Protocol error (Runtime.callFunctionOn): Target closed",
|
||||
"Navigation failed because browser has disconnected",
|
||||
"browser has disconnected",
|
||||
"Page crashed!",
|
||||
"Execution context was destroyed",
|
||||
"Cannot find context with specified id",
|
||||
"Failed to launch the browser process! TROUBLESHOOTING: https://pptr.dev/troubleshooting",
|
||||
"connect ECONNREFUSED 127.0.0.1:9222",
|
||||
])("returns true for transient error: %s", (message) => {
|
||||
expect(isTransientBrowserError(new Error(message))).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"net::ERR_NAME_NOT_RESOLVED",
|
||||
"TimeoutError: Navigation timeout of 30000 ms exceeded",
|
||||
"FONT_FETCH_FAILED: Inter",
|
||||
"Composition duration is 0",
|
||||
"SYSTEM_FONT_USED: -apple-system",
|
||||
"",
|
||||
])("returns false for non-transient error: %s", (message) => {
|
||||
expect(isTransientBrowserError(new Error(message))).toBe(false);
|
||||
});
|
||||
|
||||
it("handles non-Error values", () => {
|
||||
expect(isTransientBrowserError("Navigating frame was detached")).toBe(true);
|
||||
expect(isTransientBrowserError("some other string")).toBe(false);
|
||||
expect(isTransientBrowserError(null)).toBe(false);
|
||||
expect(isTransientBrowserError(undefined)).toBe(false);
|
||||
expect(isTransientBrowserError(42)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1932,3 +1932,25 @@ export function getCapturePerfSummary(session: CaptureSession): CapturePerfSumma
|
||||
staticDedupSkipReason: session.staticDedupSkipReason,
|
||||
};
|
||||
}
|
||||
|
||||
// ── Transient browser error classification ─────────────────────────────────
|
||||
// Puppeteer/Chrome can fail with transient errors that succeed on retry with a
|
||||
// fresh browser session. These are infrastructure-level failures (frame
|
||||
// detachment, connection drop, OOM kill, launch failure) — NOT composition bugs.
|
||||
|
||||
const TRANSIENT_BROWSER_ERROR_PATTERNS = [
|
||||
/Navigating frame was detached/i,
|
||||
/Target closed/i,
|
||||
/Session closed/i,
|
||||
/browser has disconnected/i,
|
||||
/Page crashed/i,
|
||||
/Execution context was destroyed/i,
|
||||
/Cannot find context with specified id/i,
|
||||
/Failed to launch the browser process/i,
|
||||
/ECONNREFUSED/i,
|
||||
];
|
||||
|
||||
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));
|
||||
}
|
||||
|
||||
@@ -17,6 +17,24 @@ const mockPage = {
|
||||
}),
|
||||
};
|
||||
|
||||
let initializeSessionCallCount = 0;
|
||||
let initializeSessionFailUntilAttempt = 0;
|
||||
let initializeSessionError: Error | null = null;
|
||||
let createSessionCallCount = 0;
|
||||
let createSessionFailUntilAttempt = 0;
|
||||
let createSessionError: Error | null = null;
|
||||
let closeCaptureSessionCallCount = 0;
|
||||
|
||||
function resetRetryMocks() {
|
||||
initializeSessionCallCount = 0;
|
||||
initializeSessionFailUntilAttempt = 0;
|
||||
initializeSessionError = null;
|
||||
createSessionCallCount = 0;
|
||||
createSessionFailUntilAttempt = 0;
|
||||
createSessionError = null;
|
||||
closeCaptureSessionCallCount = 0;
|
||||
}
|
||||
|
||||
mock.module("@hyperframes/engine", () => ({
|
||||
createCaptureSession: async (
|
||||
_url: string,
|
||||
@@ -25,7 +43,11 @@ mock.module("@hyperframes/engine", () => ({
|
||||
_nullArg: unknown,
|
||||
cfg: unknown,
|
||||
) => {
|
||||
createSessionCallCount++;
|
||||
capturedCfgs.push(cfg);
|
||||
if (createSessionError && createSessionCallCount <= createSessionFailUntilAttempt) {
|
||||
throw createSessionError;
|
||||
}
|
||||
return {
|
||||
isInitialized: false,
|
||||
browserConsoleBuffer: [],
|
||||
@@ -33,10 +55,24 @@ mock.module("@hyperframes/engine", () => ({
|
||||
};
|
||||
},
|
||||
initializeSession: async (session: { isInitialized: boolean }) => {
|
||||
initializeSessionCallCount++;
|
||||
if (initializeSessionError && initializeSessionCallCount <= initializeSessionFailUntilAttempt) {
|
||||
throw initializeSessionError;
|
||||
}
|
||||
session.isInitialized = true;
|
||||
},
|
||||
getCompositionDuration: async () => 5,
|
||||
closeCaptureSession: async () => {},
|
||||
closeCaptureSession: async () => {
|
||||
closeCaptureSessionCallCount++;
|
||||
},
|
||||
// Mirror of the real engine classifier. Canonical tests + pattern list
|
||||
// live in frameCapture-transientErrors.test.ts — update both if patterns change.
|
||||
isTransientBrowserError: (error: unknown) => {
|
||||
const msg = error instanceof Error ? error.message : String(error);
|
||||
return /Navigating frame was detached|Target closed|Session closed|browser has disconnected|Page crashed|Execution context was destroyed|Cannot find context with specified id|Failed to launch the browser process|ECONNREFUSED/i.test(
|
||||
msg,
|
||||
);
|
||||
},
|
||||
}));
|
||||
|
||||
mock.module("../../fileServer.js", () => ({
|
||||
@@ -221,3 +257,83 @@ describe("runProbeStage — forceScreenshot threading", () => {
|
||||
expect(capturedCfg.forceScreenshot).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runProbeStage — transient browser error retry (#1687)", () => {
|
||||
it("retries once on a transient 'Navigating frame was detached' error and succeeds", async () => {
|
||||
resetRetryMocks();
|
||||
capturedCfgs.length = 0;
|
||||
initializeSessionError = new Error("Navigating frame was detached");
|
||||
initializeSessionFailUntilAttempt = 1;
|
||||
|
||||
const { runProbeStage } = await import("./probeStage.js");
|
||||
const input = makeProbeInput({ cfgForceScreenshot: false, stageForceScreenshot: false });
|
||||
|
||||
const result = await runProbeStage(input);
|
||||
|
||||
expect(initializeSessionCallCount).toBe(2);
|
||||
expect(closeCaptureSessionCallCount).toBe(1);
|
||||
expect(result.duration).toBe(5);
|
||||
expect(result.probeSession).not.toBeNull();
|
||||
});
|
||||
|
||||
it("throws immediately on a non-transient error without retrying", async () => {
|
||||
resetRetryMocks();
|
||||
capturedCfgs.length = 0;
|
||||
initializeSessionError = new Error("FONT_FETCH_FAILED: Inter");
|
||||
initializeSessionFailUntilAttempt = 999;
|
||||
|
||||
const { runProbeStage } = await import("./probeStage.js");
|
||||
const input = makeProbeInput({ cfgForceScreenshot: false, stageForceScreenshot: false });
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await runProbeStage(input);
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
expect((caught as Error).message).toContain("FONT_FETCH_FAILED");
|
||||
expect(initializeSessionCallCount).toBe(1);
|
||||
expect(closeCaptureSessionCallCount).toBe(1);
|
||||
});
|
||||
|
||||
it("throws after exhausting retry attempts on persistent transient errors", async () => {
|
||||
resetRetryMocks();
|
||||
capturedCfgs.length = 0;
|
||||
initializeSessionError = new Error("Target closed");
|
||||
initializeSessionFailUntilAttempt = 999;
|
||||
|
||||
const { runProbeStage } = await import("./probeStage.js");
|
||||
const input = makeProbeInput({ cfgForceScreenshot: false, stageForceScreenshot: false });
|
||||
|
||||
let caught: unknown;
|
||||
try {
|
||||
await runProbeStage(input);
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
expect((caught as Error).message).toContain("Target closed");
|
||||
expect(initializeSessionCallCount).toBe(2);
|
||||
expect(closeCaptureSessionCallCount).toBe(2);
|
||||
});
|
||||
|
||||
it("retries on a transient browser LAUNCH failure (createCaptureSession throws)", async () => {
|
||||
resetRetryMocks();
|
||||
capturedCfgs.length = 0;
|
||||
createSessionError = new Error("Failed to launch the browser process!");
|
||||
createSessionFailUntilAttempt = 1;
|
||||
|
||||
const { runProbeStage } = await import("./probeStage.js");
|
||||
const input = makeProbeInput({ cfgForceScreenshot: false, stageForceScreenshot: false });
|
||||
|
||||
const result = await runProbeStage(input);
|
||||
|
||||
expect(createSessionCallCount).toBe(2);
|
||||
expect(closeCaptureSessionCallCount).toBe(0);
|
||||
expect(result.duration).toBe(5);
|
||||
expect(result.probeSession).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,9 +33,11 @@ import {
|
||||
type CaptureOptions,
|
||||
type CaptureSession,
|
||||
type EngineConfig,
|
||||
closeCaptureSession,
|
||||
createCaptureSession,
|
||||
getCompositionDuration,
|
||||
initializeSession,
|
||||
isTransientBrowserError,
|
||||
} from "@hyperframes/engine";
|
||||
import { fpsToNumber } from "@hyperframes/core";
|
||||
import type { CompiledComposition } from "../../htmlCompiler.js";
|
||||
@@ -175,35 +177,78 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
quality: needsAlpha ? undefined : 80,
|
||||
deviceScaleFactor,
|
||||
};
|
||||
log.info("Browser launched, creating capture session...");
|
||||
probeSession = await createCaptureSession(
|
||||
fileServer.url,
|
||||
join(workDir, "probe"),
|
||||
captureOpts,
|
||||
null,
|
||||
probeCfg,
|
||||
);
|
||||
log.info("Waiting for composition to initialize...");
|
||||
const initStart = Date.now();
|
||||
const heartbeat = setInterval(() => {
|
||||
const elapsed = ((Date.now() - initStart) / 1000).toFixed(1);
|
||||
log.info(`Still waiting for browser initialization... (${elapsed}s elapsed)`);
|
||||
}, 30_000);
|
||||
try {
|
||||
await initializeSession(probeSession);
|
||||
} finally {
|
||||
clearInterval(heartbeat);
|
||||
|
||||
const PROBE_MAX_ATTEMPTS = 2;
|
||||
for (let attempt = 1; attempt <= PROBE_MAX_ATTEMPTS; attempt++) {
|
||||
const attemptStart = Date.now();
|
||||
try {
|
||||
log.info("Creating capture session...", { attempt, maxAttempts: PROBE_MAX_ATTEMPTS });
|
||||
probeSession = await createCaptureSession(
|
||||
fileServer.url,
|
||||
join(workDir, "probe"),
|
||||
captureOpts,
|
||||
null,
|
||||
probeCfg,
|
||||
);
|
||||
log.info("Waiting for composition to initialize...", { attempt });
|
||||
const heartbeat = setInterval(() => {
|
||||
const elapsed = ((Date.now() - attemptStart) / 1000).toFixed(1);
|
||||
log.info(`Still waiting for browser initialization... (${elapsed}s elapsed)`);
|
||||
}, 30_000);
|
||||
try {
|
||||
await initializeSession(probeSession);
|
||||
} finally {
|
||||
clearInterval(heartbeat);
|
||||
}
|
||||
} catch (err) {
|
||||
const isTransient = isTransientBrowserError(err);
|
||||
const errMsg = err instanceof Error ? err.message : String(err);
|
||||
log.warn("Browser probe attempt failed", {
|
||||
attempt,
|
||||
maxAttempts: PROBE_MAX_ATTEMPTS,
|
||||
isTransient,
|
||||
error: errMsg,
|
||||
elapsedMs: Date.now() - attemptStart,
|
||||
});
|
||||
|
||||
if (probeSession) {
|
||||
try {
|
||||
await closeCaptureSession(probeSession);
|
||||
} catch (closeErr) {
|
||||
log.warn("Failed to close crashed probe session", {
|
||||
error: closeErr instanceof Error ? closeErr.message : String(closeErr),
|
||||
});
|
||||
}
|
||||
probeSession = null;
|
||||
}
|
||||
|
||||
if (isTransient && attempt < PROBE_MAX_ATTEMPTS) {
|
||||
log.info("Retrying with a fresh browser session...", {
|
||||
attempt: attempt + 1,
|
||||
maxAttempts: PROBE_MAX_ATTEMPTS,
|
||||
});
|
||||
assertNotAborted();
|
||||
continue;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
log.info("Composition ready", {
|
||||
attempt,
|
||||
initMs: Date.now() - attemptStart,
|
||||
});
|
||||
break;
|
||||
}
|
||||
log.info("Composition ready", {
|
||||
initMs: Date.now() - initStart,
|
||||
});
|
||||
assertNotAborted();
|
||||
lastBrowserConsole = probeSession.browserConsoleBuffer;
|
||||
// After the retry loop, probeSession is guaranteed non-null (the loop
|
||||
// either breaks with a valid session or throws on the last attempt).
|
||||
const session = probeSession!;
|
||||
probeSession = session;
|
||||
lastBrowserConsole = session.browserConsoleBuffer;
|
||||
|
||||
// Discover root composition duration
|
||||
if (composition.duration <= 0) {
|
||||
log.info("Discovering composition duration...");
|
||||
const discoveredDuration = await getCompositionDuration(probeSession);
|
||||
const discoveredDuration = await getCompositionDuration(session);
|
||||
assertNotAborted();
|
||||
log.info("Probed composition duration from browser", {
|
||||
discoveredDuration,
|
||||
@@ -219,7 +264,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
// Resolve unresolved composition durations via window.__timelines
|
||||
if (compiled.unresolvedCompositions.length > 0) {
|
||||
const resolutions = await resolveCompositionDurations(
|
||||
probeSession.page,
|
||||
session.page,
|
||||
compiled.unresolvedCompositions,
|
||||
);
|
||||
assertNotAborted();
|
||||
@@ -241,7 +286,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
|
||||
// Discover media elements from browser DOM (catches dynamically-set src)
|
||||
log.info("Discovering media assets from browser DOM...");
|
||||
const browserMedia = await discoverMediaFromBrowser(probeSession.page);
|
||||
const browserMedia = await discoverMediaFromBrowser(session.page);
|
||||
assertNotAborted();
|
||||
if (browserMedia.length > 0) {
|
||||
const existingVideoIds = new Set(composition.videos.map((v) => v.id));
|
||||
@@ -356,7 +401,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
audioCount: composition.audios.length,
|
||||
});
|
||||
const automation = await discoverAudioVolumeAutomationFromTimeline(
|
||||
probeSession.page,
|
||||
session.page,
|
||||
composition.audios.map((audio) => audio.id),
|
||||
composition.duration,
|
||||
fpsToNumber(job.config.fps),
|
||||
@@ -382,7 +427,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
|
||||
videoCount: composition.videos.length,
|
||||
});
|
||||
const visibilityWindows = await discoverVideoVisibilityFromTimeline(
|
||||
probeSession.page,
|
||||
session.page,
|
||||
composition.duration,
|
||||
);
|
||||
assertNotAborted();
|
||||
|
||||
Reference in New Issue
Block a user