Skip to content

Commit f500a42

Browse files
authored
fix(producer): type video extraction failures (#2776)
## Summary - classify per-source video download/probe/decode/extraction failures with a bounded taxonomy and safe producer-facing summaries - add candidate-only, at-most-one transient retry with cleanup and retry telemetry - preserve default engine/producer behavior when the policy is off - carry allowlisted extraction error codes through blocking JSON and SSE responses ## Stack Depends on #2774 for atomic remote downloads and its single owned download retry. This PR is intentionally based on `fix/atomic-video-download-retry`; rebase/change the base to `main` after #2774 merges. ## Default compatibility `HF_VIDEO_EXTRACTION_FAILURE_MODE` defaults to `off` and forces `maxTransientRetries=0`. With the feature off: - metadata probe failures keep the legacy Promise rejection - grouped extraction keeps the existing grouped-to-direct fallback - no new producer failure gate is enforced - render-plan schema, Plan v1 artifacts, chunk routing, and distributed execution are unchanged Typed metadata aggregation is explicit and enabled only by the candidate enforce lane. ## Retry ownership - remote downloads: exactly one retry owned by #2774 - metadata/FFmpeg extraction: at most one retry only when `HF_VIDEO_EXTRACTION_MAX_RETRIES=1` - invalid, missing, rejected, out-of-range, zero-output, cancellation, and unknown/internal failures do not retry - non-finite or invalid runtime retry budgets fail closed to zero - the superset optimization is never retried; on failure it preserves direct-member fallback, and only the individual ranges can use the bounded retry - retry counters increment when a retry is scheduled, including exhausted retries The internal sidecar and Experiment Framework must treat both exhausted stage codes as workflow-terminal after the producer-local budget. Candidate enforcement must not be enabled until those companion mappings are deployed, or Temporal can multiply producer attempts. ## Failure contract - `VIDEO_SOURCE_UNRENDERABLE`: at least one deterministic/unknown source failure - `VIDEO_EXTRACTION_FAILED`: all source failures are transient but the producer-local budget is exhausted Only the allowlisted code and kind/count summaries cross JSON/SSE. Raw diagnostics remain engine-local because they may contain signed URLs or local paths. ## Rollout 1. merge and deploy with stable/candidate both `off` 2. candidate `observe`, retries 0 3. candidate `observe`, retries 1 4. deploy internal + EF terminal transport mappings 5. candidate `enforce`, retries 1 6. keep stable off until success delta, retry counts, extraction latency, CPU/disk, and queue backlog are acceptable ## Validation - engine focused suites: 105 passed - producer focused suites: 15 passed - full engine suite: 1,176 passed, 3 skipped - full producer unit lane: 32 Vitest files / 393 tests plus all classified Bun unit tests - engine and producer typechecks passed - oxlint, oxfmt, Fallow, tracked-artifact, and commit hooks passed - independent review: approved for merge default-off; candidate enforcement held on companion transport rollout
2 parents 814f9cd + 33ca1de commit f500a42

13 files changed

Lines changed: 1102 additions & 63 deletions

packages/engine/src/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,18 @@ export {
184184
createFrameLookupTable,
185185
FrameLookupTable,
186186
analyzeClipMediaFit,
187+
classifyVideoExtractionError,
188+
isVideoSourceExtractionError,
189+
runVideoExtractionWithRetry,
190+
VideoSourceExtractionError,
187191
type VideoElement,
188192
type ImageElement,
189193
type ExtractedFrames,
190194
type ExtractionOptions,
191195
type ExtractionResult,
192196
type ExtractionPhaseBreakdown,
197+
type VideoExtractionFailure,
198+
type VideoExtractionFailureKind,
193199
type VideoFrameFormat,
194200
VIDEO_FRAME_FORMATS,
195201
isVideoFrameFormat,
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { describe, expect, it } from "vitest";
2+
import { classifyFfmpegSpawnError } from "./videoFrameExtractor.js";
3+
4+
describe("classifyFfmpegSpawnError", () => {
5+
it.each(["ENOENT", "EACCES", "ENOEXEC", "UNKNOWN"])(
6+
"keeps deterministic launch failure %s terminal",
7+
(code) => {
8+
expect(classifyFfmpegSpawnError(Object.assign(new Error(code), { code }))).toMatchObject({
9+
retryable: false,
10+
});
11+
},
12+
);
13+
14+
it.each(["EAGAIN", "EMFILE", "ENFILE"])("retries known transient launch failure %s", (code) => {
15+
expect(classifyFfmpegSpawnError(Object.assign(new Error(code), { code }))).toMatchObject({
16+
kind: "ffmpeg_transient",
17+
retryable: true,
18+
});
19+
});
20+
});

packages/engine/src/services/videoFrameExtractor.test.ts

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// fallow-ignore-file code-duplication
12
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
23
import {
34
existsSync,
@@ -25,6 +26,9 @@ import {
2526
decoderForCodec,
2627
getFrameAtTime,
2728
analyzeClipMediaFit,
29+
classifyVideoExtractionError,
30+
runVideoExtractionWithRetry,
31+
VideoSourceExtractionError,
2832
type VideoElement,
2933
type ExtractedFrames,
3034
type ExtractionResult,
@@ -41,6 +45,120 @@ import { COMPLETE_SENTINEL, GC_MARKER, SCHEMA_PREFIX } from "./extractionCache.j
4145
// synthesized VFR fixture.
4246
const HAS_FFMPEG = spawnSync("ffmpeg", ["-version"]).status === 0;
4347

48+
describe("video extraction failure taxonomy and bounded retry", () => {
49+
it("classifies missing and transient HTTP sources without exposing retry ambiguity", () => {
50+
expect(classifyVideoExtractionError(new Error("HTTP 404: Not Found"))).toMatchObject({
51+
kind: "download_not_found",
52+
retryable: false,
53+
});
54+
expect(classifyVideoExtractionError(new Error("HTTP 503: Service Unavailable"))).toMatchObject({
55+
kind: "download_transient",
56+
retryable: true,
57+
});
58+
});
59+
60+
it("retries one transient failure, cleaning partial output before the retry", async () => {
61+
const retryDir = mkdtempSync(join(tmpdir(), "hf-extract-retry-"));
62+
const partialPath = join(retryDir, "frame-00001.jpg");
63+
let attempts = 0;
64+
try {
65+
const outcome = await runVideoExtractionWithRetry(
66+
async () => {
67+
attempts += 1;
68+
if (attempts === 1) {
69+
writeFileSync(partialPath, "partial");
70+
throw new VideoSourceExtractionError(
71+
"ffmpeg_timeout",
72+
true,
73+
"Video frame extraction timed out",
74+
);
75+
}
76+
expect(existsSync(partialPath)).toBe(false);
77+
return "frames";
78+
},
79+
{
80+
maxTransientRetries: 1,
81+
onRetry: () => {
82+
rmSync(retryDir, { recursive: true, force: true });
83+
mkdirSync(retryDir, { recursive: true });
84+
},
85+
},
86+
);
87+
88+
expect(outcome).toEqual({ result: "frames", retries: 1 });
89+
expect(attempts).toBe(2);
90+
} finally {
91+
rmSync(retryDir, { recursive: true, force: true });
92+
}
93+
});
94+
95+
it("does not retry deterministic or caller-aborted failures", async () => {
96+
let deterministicAttempts = 0;
97+
await expect(
98+
runVideoExtractionWithRetry(async () => {
99+
deterministicAttempts += 1;
100+
throw new VideoSourceExtractionError(
101+
"zero_output",
102+
false,
103+
"Video source produced no decodable frames",
104+
);
105+
}),
106+
).rejects.toMatchObject({ kind: "zero_output", retryable: false });
107+
expect(deterministicAttempts).toBe(1);
108+
109+
const controller = new AbortController();
110+
controller.abort();
111+
let abortedAttempts = 0;
112+
await expect(
113+
runVideoExtractionWithRetry(
114+
async () => {
115+
abortedAttempts += 1;
116+
throw new VideoSourceExtractionError(
117+
"download_transient",
118+
true,
119+
"Video source download failed transiently",
120+
);
121+
},
122+
{ signal: controller.signal },
123+
),
124+
).rejects.toMatchObject({ kind: "cancelled", retryable: false });
125+
expect(abortedAttempts).toBe(0);
126+
});
127+
128+
it("does not retry transient extraction failures unless the caller opts in", async () => {
129+
let attempts = 0;
130+
await expect(
131+
runVideoExtractionWithRetry(async () => {
132+
attempts += 1;
133+
throw new VideoSourceExtractionError(
134+
"ffmpeg_timeout",
135+
true,
136+
"Video frame extraction timed out",
137+
);
138+
}),
139+
).rejects.toMatchObject({ kind: "ffmpeg_timeout", retryable: true });
140+
expect(attempts).toBe(1);
141+
});
142+
143+
it("fails closed to zero retries for a non-finite runtime retry budget", async () => {
144+
let attempts = 0;
145+
await expect(
146+
runVideoExtractionWithRetry(
147+
async () => {
148+
attempts += 1;
149+
throw new VideoSourceExtractionError(
150+
"ffmpeg_timeout",
151+
true,
152+
"Video frame extraction timed out",
153+
);
154+
},
155+
{ maxTransientRetries: Number.NaN },
156+
),
157+
).rejects.toMatchObject({ kind: "ffmpeg_timeout", retryable: true });
158+
expect(attempts).toBe(1);
159+
});
160+
});
161+
44162
// Codec-based alpha defaulting replaces tag-based detection (the
45163
// alpha_mode/ALPHA_MODE case bug — see ffprobe.test.ts for the regression
46164
// pin on that). The extractor uses these helpers for two decisions:
@@ -966,6 +1084,45 @@ describe.skipIf(!HAS_FFMPEG)("extractAllVideoFrames on a VFR source", () => {
9661084
return src;
9671085
}
9681086

1087+
it("rejects a media start beyond source duration before invoking FFmpeg", async () => {
1088+
const src = await synthCfrClip("zero-output-src.mp4", 1);
1089+
const outputDir = join(FIXTURE_DIR, "out-zero-output");
1090+
await expect(
1091+
extractVideoFramesRange(src, "past-eof", 2, 1, { fps: 30, outputDir }),
1092+
).rejects.toMatchObject({
1093+
kind: "media_start_out_of_range",
1094+
retryable: false,
1095+
});
1096+
}, 60_000);
1097+
1098+
it("preserves legacy metadata rejection unless typed aggregation is explicitly enabled", async () => {
1099+
const src = join(FIXTURE_DIR, "invalid-probe.mp4");
1100+
writeFileSync(src, "not a media container");
1101+
const video = cfrClipElement("invalid-probe", src, 1);
1102+
1103+
await expect(
1104+
extractAllVideoFrames([video], FIXTURE_DIR, {
1105+
fps: 30,
1106+
outputDir: join(FIXTURE_DIR, "out-invalid-probe-legacy"),
1107+
}),
1108+
).rejects.toThrow();
1109+
1110+
const collected = await extractAllVideoFrames([video], FIXTURE_DIR, {
1111+
fps: 30,
1112+
outputDir: join(FIXTURE_DIR, "out-invalid-probe-typed"),
1113+
collectProbeFailures: true,
1114+
});
1115+
expect(collected.success).toBe(false);
1116+
expect(collected.extracted).toEqual([]);
1117+
expect(collected.errors).toEqual([
1118+
expect.objectContaining({
1119+
videoId: "invalid-probe",
1120+
kind: "invalid_media",
1121+
retryable: false,
1122+
}),
1123+
]);
1124+
}, 60_000);
1125+
9691126
async function synthHdrTaggedClip(name: string, durationSeconds: number): Promise<string> {
9701127
const src = join(FIXTURE_DIR, name);
9711128
const synth = await runFfmpeg([

0 commit comments

Comments
 (0)