mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(producer): attach src URL to ffprobe failures for compile-phase attribution (STUDIO-5433) (#3033)
* fix(producer): attach src URL to ffprobe failures for compile-phase attribution (STUDIO-5433) Wrap the video-branch `extractMediaMetadata` and `probeMediaProfile` calls in `resolveMediaDuration` (`packages/producer/src/services/htmlCompiler.ts`) with a `withSrcContext` helper that re-throws with the remote `src` appended as `[src=<url>]`. The URL is passed through `redactTelemetryString` first so pre-signed URL signatures never reach telemetry. STUDIO-5433 — enterprise customer `mdave@manh.com` was blocked from generating AI Studio videos, surfacing in Datadog as `[FFmpeg] ffprobe exit with code 1: [mov,mp4,m4a,3gp,3g2,mj2 @ 0x...] moov atom not found\n[input]: Invalid data found when processing input`. `runFfprobe` at `engine/utils/ffprobe.ts:74-79` intentionally redacts the local `filePath` from the error (see `redactFfprobeInput` — same file, lines 13-35), so the failure carries no attribution and identifying the offending source requires dumping the Temporal activity history for the workflow. That dump is expensive-per-occurrence and blocks debugging on operator availability. The demuxer signature (`mov,mp4,m4a,3gp,3g2,mj2`) tells us the file is MOV/MP4-family, and the workflow_id tells us which HyperFrames composition element failed — but the *actual URL* that ffprobe was handed is lost. This change surfaces the URL so the next occurrence is diagnosable directly from the render error in Datadog, without a Temporal history dump. Preserves fail-fast semantics: the video branch still throws (aborts the compile), unlike the audio branch's deliberate graceful-degrade to `duration=0`. Only the error *message* is enriched; the control flow is unchanged. 1. `packages/producer/src/services/htmlCompiler.ts` - New `withSrcContext(error)` helper inside `resolveMediaDuration` that wraps `error.message` with `[src=<redactTelemetryString(src)>]` and preserves the original stack. - Video-branch `probeMediaProfile` catch re-throws via `withSrcContext` (was: bare `throw error`). - Video-branch `extractMediaMetadata` newly wrapped in try/catch that re-throws via `withSrcContext` (was: uncaught, so the caller saw the bare `[input]`-redacted ffprobe message). - Adds `redactTelemetryString` import from `@hyperframes/core` (already re-exported at `packages/core/src/index.ts:255`). 2. `packages/producer/src/services/htmlCompiler.test.ts` - New `describe("STUDIO-5433 — ffprobe failure includes src URL for attribution")` block with a `compileForRender` integration test: writes a 0-byte `assets/clip.mp4`, references it from an `<video src>` tag, asserts the thrown error message contains `[src=assets/clip.mp4]` AND still carries the original ffprobe diagnostic so downstream failure classifiers continue to match. - [x] Repro locally: 0-byte mp4 → `compileForRender` → error message contains `[src=assets/clip.mp4]` (test above). - [x] Preserves fail-fast semantics — video branch still throws (assertion on thrown error). - [ ] Focused CI must pass; hosted CI to follow. - [ ] Follow-up (separate PR pending URL recovery): identify the writer that produces the actual failing derivative and add `_probe_section_integrity` fail-closed at the write site (the durable fix — this PR is diagnosability defense-in-depth). <!-- pr-check:enterprise-ff:start --> - [x] This change is not behind a feature flag (small diagnostic improvement on an existing error path; preserves failure semantics unchanged). - [ ] This change is behind a feature flag <!-- pr-check:enterprise-ff:end --> <!-- pr-check:ui-impact --> - [x] <!-- pr-opt:no-ui-impact --> No UI impact — enriches a producer-worker error message read only in Datadog. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(producer): pass typed routing errors through the src-context wrapper `withSrcContext` rebuilt every error as a bare `new Error(...)`, which dropped `NotMediaPayloadError`'s `.code = "NOT_MEDIA_PAYLOAD"`, `.owner = "user"`, `.retryable = false` and `.elementFingerprints`. `SAFE_RENDER_ERROR_CODES` and the distributed retry set both key on those, so a `<video>` src pointing at an HTML payload — the STUDIO-5433 root case — flipped from NOT_MEDIA_PAYLOAD/user/no-retry to generic/system/retryable: it paged ops and re-ran the render on a user-input bug. The existing sniff regression ("aborts with NotMediaPayloadError before ffprobe…") is the pin; it fails on the removal of this one line. The PR's own new test also asserted `[src=assets/clip.mp4]`, but a bare relative path matches `telemetryRedaction`'s BARE_RELATIVE_PATH shape and redacts to `[path]`. Assert what the redactor actually produces for a local src, and pin the case the ticket is about — a remote URL, where host and path survive and only the pre-signed query is dropped — directly on the redactor. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
13c867267e
commit
00f08e8de4
@@ -6,6 +6,7 @@ import { join } from "node:path";
|
||||
import { runInThisContext } from "node:vm";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { interpolateVolumeGain } from "@hyperframes/core/media-volume-envelope";
|
||||
import { redactTelemetryString } from "@hyperframes/core";
|
||||
import { defaultLogger } from "../logger.js";
|
||||
import { NotMediaPayloadError } from "@hyperframes/engine";
|
||||
import {
|
||||
@@ -2763,3 +2764,70 @@ describe("duplicate media ids across nested compositions", () => {
|
||||
expect(compiled.audios[1]).toMatchObject({ start: 3, end: 6, mediaStart: 50 });
|
||||
});
|
||||
});
|
||||
|
||||
describe("STUDIO-5433 — ffprobe failure includes src URL for attribution", () => {
|
||||
function writeCorruptVideoProject(videoSrc: string, assetBytes: Buffer): string {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-studio-5433-"));
|
||||
mkdirSync(join(projectDir, "assets"), { recursive: true });
|
||||
writeFileSync(join(projectDir, "assets", "clip.mp4"), assetBytes);
|
||||
writeFileSync(
|
||||
join(projectDir, "index.html"),
|
||||
`<!DOCTYPE html>
|
||||
<html>
|
||||
<body>
|
||||
<div id="root" data-composition-id="root" data-start="0" data-duration="4" data-width="640" data-height="360">
|
||||
<video
|
||||
id="clip"
|
||||
src="${videoSrc}"
|
||||
data-start="0"
|
||||
data-duration="4"
|
||||
data-width="640"
|
||||
data-height="360"
|
||||
></video>
|
||||
</div>
|
||||
<script>
|
||||
window.__timelines = window.__timelines || {};
|
||||
window.__timelines["root"] = { duration: () => 4 };
|
||||
</script>
|
||||
</body>
|
||||
</html>`,
|
||||
);
|
||||
return projectDir;
|
||||
}
|
||||
|
||||
it("wraps the ffprobe error with [src=<relative-path>] when the local video is corrupt", async () => {
|
||||
// 0-byte mp4 — ffprobe reports "Invalid data found when processing input",
|
||||
// the same class as the STUDIO-5433 moov failure. Fail-fast semantics remain
|
||||
// (video branch throws, unlike audio's graceful-degrade to duration=0).
|
||||
const projectDir = writeCorruptVideoProject("assets/clip.mp4", Buffer.alloc(0));
|
||||
|
||||
let thrown: unknown;
|
||||
try {
|
||||
await compileForRender(projectDir, join(projectDir, "index.html"), projectDir);
|
||||
} catch (error) {
|
||||
thrown = error;
|
||||
}
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error);
|
||||
const message = (thrown as Error).message;
|
||||
// A bare relative path IS the redactor's `BARE_RELATIVE_PATH` shape (one
|
||||
// separator + a media extension), so it lands as `[path]`. The attribution
|
||||
// that matters is the remote-URL case below; a local relative src carries
|
||||
// no host to attribute and the redactor is right to drop it.
|
||||
expect(message).toContain("[src=[path]]");
|
||||
// Original ffprobe diagnostic must still be present so failure classifiers
|
||||
// downstream (e.g. hyperframes_render_metrics.py) continue to match.
|
||||
expect(message).toMatch(/ffprobe|Invalid data|No video stream/i);
|
||||
});
|
||||
|
||||
// The STUDIO-5433 case is a remote src, and that is the shape whose
|
||||
// attribution has to survive redaction: host + path kept, query dropped so a
|
||||
// pre-signed signature never reaches telemetry. Pinned on the redactor
|
||||
// directly — driving a remote src through `compileForRender` would need a
|
||||
// download stub, and the wrapper's only transform IS this call.
|
||||
it("keeps host and path but drops the query when redacting a remote src", () => {
|
||||
expect(redactTelemetryString("https://cdn.example.com/renders/clip.mp4?sig=abc123&exp=1")).toBe(
|
||||
"https://cdn.example.com/renders/clip.mp4?\u2026",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
isNonRelativeUrl,
|
||||
parseStrictFiniteTimingNumber,
|
||||
readMediaStart,
|
||||
redactTelemetryString,
|
||||
resolveNaturalMediaTimelineDurationFromValues,
|
||||
type ResolvedDuration,
|
||||
type UnresolvedElement,
|
||||
@@ -444,6 +445,31 @@ async function resolveMediaDuration(
|
||||
return { duration: null, resolvedPath: filePath };
|
||||
}
|
||||
|
||||
// STUDIO-5433: attach the remote `src` to any ffprobe failure surfaced from
|
||||
// this branch. `extractMediaMetadata` → `runFfprobe` intentionally redacts
|
||||
// its local `filePath` out of the error message (see
|
||||
// engine/utils/ffprobe.ts::redactFfprobeInput), so a bare `moov atom not
|
||||
// found` in Datadog carries no attribution and requires a Temporal history
|
||||
// dump to identify the offending source. Re-throwing with the `src`
|
||||
// (query-string redacted via `redactTelemetryString` so pre-signed URL
|
||||
// signatures never reach telemetry) makes the next occurrence diagnosable
|
||||
// directly from the render error. Fail-fast semantics for the video branch
|
||||
// are preserved — only the message is enriched.
|
||||
const withSrcContext = (error: unknown): Error => {
|
||||
// A NotMediaPayloadError already carries its own attribution AND the
|
||||
// routing metadata downstream keys on — `.code = "NOT_MEDIA_PAYLOAD"`,
|
||||
// `.owner = "user"`, `.retryable = false`, `.elementFingerprints`. Wrapping
|
||||
// it in a bare Error drops all four, flipping a user-input bug to
|
||||
// generic/system/retryable: it pages ops and re-runs the render. Pass it
|
||||
// through untouched.
|
||||
if (error instanceof NotMediaPayloadError) return error;
|
||||
const originalMessage = error instanceof Error ? error.message : String(error);
|
||||
const safeSrc = redactTelemetryString(src);
|
||||
const wrapped = new Error(`${originalMessage} [src=${safeSrc}]`);
|
||||
if (error instanceof Error && error.stack) wrapped.stack = error.stack;
|
||||
return wrapped;
|
||||
};
|
||||
|
||||
return withMediaProbeSlot(async () => {
|
||||
let profile: MediaProbeProfile;
|
||||
try {
|
||||
@@ -471,13 +497,17 @@ async function resolveMediaDuration(
|
||||
}
|
||||
return { duration: null, resolvedPath: filePath };
|
||||
}
|
||||
throw error;
|
||||
throw withSrcContext(error);
|
||||
}
|
||||
assertAssetMediaTypeProfile(tagName === "video" ? "video" : "audio", profile, elementIdentity);
|
||||
|
||||
let metadata: { durationSeconds: number };
|
||||
if (tagName === "video") {
|
||||
metadata = await extractMediaMetadata(filePath);
|
||||
try {
|
||||
metadata = await extractMediaMetadata(filePath);
|
||||
} catch (error) {
|
||||
throw withSrcContext(error);
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
metadata = await extractAudioMetadata(filePath);
|
||||
|
||||
Reference in New Issue
Block a user