mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(producer,engine,cli): oom wrapping, cancellation, fallback-reason gaps
Three defects found by max-effort code review of this branch: 1. The Bun OOM exact-match regex was defeated by this codebase's own parallel-worker error wrapping. executeParallelCapture/formatWorkerFailure (parallelCoordinator.ts) always wrap a worker's error as "Worker N: <message>", optionally suffixed and joined with other workers' segments, all prefixed "[Parallel] Capture failed: ". That wrapping defeated the exact-message check for exactly the cohort (deParallelRouter routed, N separate Chrome processes) the OOM-drops-to-1 fix targets — a real OOM there would retry at the SAME worker count instead of dropping to 1. Added a second pattern that recovers the signal by requiring "out of memory" appear as the WHOLE content of a "Worker N: ..." segment (bounded by end-of-string/"; "), preserving the same exact-match property (no bare substring match) while surviving the wrapping. Verified against the real wrapping logic, not a hand-typed guess at its shape. 2. shouldRetryViaPinnedFallback didn't exclude cancellation, so aborting a render mid-capture on the pinned router/inversion cohort would detour through spawning a fresh encoder/capture session before the outer catch's RenderCancelledError branch ended the render — delaying "stop" with a pointless resource spin-up/tear-down. Added an isCancellation param (checked first, before isVerifyError) using the same `err instanceof RenderCancelledError || abortSignal?.aborted` check the outer catch already uses. 3. deFallbackReason (this PR's new "oom"/"capture_error" values) was set locally but never mirrored into RenderCaptureObservability alongside deSelfVerifyFallback, so a render that fails AFTER a fallback attempt (perfSummary never built) was indistinguishable in render_error telemetry from one that never attempted any fallback — undercutting the "how often does the OOM retry fire on a render that still ultimately fails" question this branch exists to answer. Threaded through RenderCaptureObservability → RenderObservabilityTelemetryPayload → renderObservabilityTelemetryPayload, mirroring the existing deSelfVerifyFallback plumbing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
b3f244a7e9
commit
a355fb2f6b
@@ -75,6 +75,28 @@ describe("render telemetry events", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("carries de_fallback_reason on render_error so a render that fails AFTER an OOM-triggered fallback attempt is distinguishable from one that never attempted a fallback", () => {
|
||||
trackRenderError({
|
||||
fps: 30,
|
||||
quality: "standard",
|
||||
docker: false,
|
||||
errorMessage: "worker crashed again after fallback",
|
||||
captureDeParallelRouter: "reverted",
|
||||
captureDeSelfVerifyFallback: false,
|
||||
captureDeFallbackReason: "oom",
|
||||
});
|
||||
|
||||
expect(trackEvent).toHaveBeenCalledWith(
|
||||
"render_error",
|
||||
expect.objectContaining({
|
||||
de_parallel_router: "reverted",
|
||||
de_self_verify_fallback: false,
|
||||
de_fallback_reason: "oom",
|
||||
}),
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it("prefers the explicit perfSummary-sourced de_worker_inversion over the capture-observability fallback on render_complete", () => {
|
||||
trackRenderComplete({
|
||||
durationMs: 1000,
|
||||
|
||||
@@ -46,6 +46,7 @@ export interface RenderObservabilityTelemetryPayload {
|
||||
captureDeParallelRouter?: string;
|
||||
captureDePreRouterWorkers?: number;
|
||||
captureDeSelfVerifyFallback?: boolean;
|
||||
captureDeFallbackReason?: string;
|
||||
observabilityExtractVideoCount?: number;
|
||||
observabilityExtractedVideoCount?: number;
|
||||
observabilityExtractTotalFrames?: number;
|
||||
@@ -96,6 +97,7 @@ function renderObservabilityEventProperties(props: RenderObservabilityTelemetryP
|
||||
de_parallel_router: props.captureDeParallelRouter,
|
||||
de_pre_router_workers: props.captureDePreRouterWorkers,
|
||||
de_self_verify_fallback: props.captureDeSelfVerifyFallback,
|
||||
de_fallback_reason: props.captureDeFallbackReason,
|
||||
observability_extract_video_count: props.observabilityExtractVideoCount,
|
||||
observability_extracted_video_count: props.observabilityExtractedVideoCount,
|
||||
observability_extract_total_frames: props.observabilityExtractTotalFrames,
|
||||
|
||||
@@ -65,4 +65,20 @@ describe("renderObservabilityTelemetryPayload — DE inversion/router cohort (fa
|
||||
);
|
||||
expect(payload.captureDeSelfVerifyFallback).toBe(true);
|
||||
});
|
||||
|
||||
it("carries deFallbackReason so a render that fails AFTER an OOM-triggered fallback attempt is distinguishable from one that never attempted a fallback", () => {
|
||||
const payload = renderObservabilityTelemetryPayload(
|
||||
makeSummary({
|
||||
deParallelRouter: "routed",
|
||||
deSelfVerifyFallback: false,
|
||||
deFallbackReason: "oom",
|
||||
}),
|
||||
);
|
||||
expect(payload.captureDeFallbackReason).toBe("oom");
|
||||
});
|
||||
|
||||
it("leaves deFallbackReason undefined when no fallback was ever attempted", () => {
|
||||
const payload = renderObservabilityTelemetryPayload(makeSummary({}));
|
||||
expect(payload.captureDeFallbackReason).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,6 +45,7 @@ export function renderObservabilityTelemetryPayload(
|
||||
captureDeParallelRouter: capture.deParallelRouter,
|
||||
captureDePreRouterWorkers: capture.dePreRouterWorkers,
|
||||
captureDeSelfVerifyFallback: capture.deSelfVerifyFallback,
|
||||
captureDeFallbackReason: capture.deFallbackReason,
|
||||
observabilityExtractVideoCount: extraction?.videoCount,
|
||||
observabilityExtractedVideoCount: extraction?.extractedVideoCount,
|
||||
observabilityExtractTotalFrames: extraction?.totalFramesExtracted,
|
||||
|
||||
Reference in New Issue
Block a user