mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 15:20:13 +00:00
fix(producer): classify JSON error bodies as non-media sources too
A source that answers with a JSON error body still reached ffprobe and
produced `moov atom not found`. Replicate returns
`{"detail": "requested file not found"}` for a dead asset, and a gateway
in front of it can relay that body with a success status.
The sniff now treats `<`, `{`, or `[` as the opening byte of a text
document. No supported container starts with any of them, so this is the
same trade as before: three bytes instead of an allowlist that grows one
entry per payload shape observed in production.
Renamed accordingly, since the class now covers JSON as well as markup:
MARKUP_NOT_MEDIA -> NOT_MEDIA_PAYLOAD, MarkupNotMediaError ->
NotMediaPayloadError, markupPayload.ts -> notMediaPayload.ts. Registry
entries in the Lambda name map, the CDK and SAM plan lists, the Cloud Run
set, and SAFE_RENDER_ERROR_CODES move with it.
Also documents the reachability boundary on the error class: only a 2xx
response gets here. `downloadToTemp` rejects 404/410 as `http_not_found`
before writing a byte, and every ffprobe input is local because
videoFrameExtractor downloads http srcs first. So the shapes this
classifies are soft-404 and interstitial HTML, S3/CloudFront error
documents, and JSON API error bodies -- each served with a success
status. A genuine 404 surfaces as a download failure, not as this error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f3689c1481
commit
349c066a83
+4
-4
@@ -1,17 +1,17 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import { NotMediaPayloadError } from "@hyperframes/engine";
|
||||
import { extractSafeRenderErrorMetadata } from "./server.js";
|
||||
|
||||
// Kept out of server.errorCode.test.ts so that suite keeps exactly one typed
|
||||
// error class in scope: two same-shaped classes there defeat the dead-code
|
||||
// analyzer's member resolution and it reports the sibling's fields as unused.
|
||||
describe("extractSafeRenderErrorMetadata — markup payloads", () => {
|
||||
describe("extractSafeRenderErrorMetadata — non-media payloads", () => {
|
||||
it("transports the code, owner, and retry policy", () => {
|
||||
// Without the SAFE_RENDER_ERROR_CODES entry the API emits
|
||||
// `errorCode: undefined` and the failure is indistinguishable from an
|
||||
// untyped crash — as unroutable as the `moov atom not found` it replaces.
|
||||
expect(extractSafeRenderErrorMetadata(new MarkupNotMediaError(["0123456789abcdef"]))).toEqual({
|
||||
errorCode: "MARKUP_NOT_MEDIA",
|
||||
expect(extractSafeRenderErrorMetadata(new NotMediaPayloadError(["0123456789abcdef"]))).toEqual({
|
||||
errorCode: "NOT_MEDIA_PAYLOAD",
|
||||
errorOwner: "user",
|
||||
retryable: false,
|
||||
});
|
||||
@@ -121,7 +121,7 @@ interface PreparedRenderInput {
|
||||
const DEFAULT_SERVER_FPS = { num: 30, den: 1 } as const;
|
||||
const SAFE_RENDER_ERROR_CODES = new Set<string>([
|
||||
"ASSET_MEDIA_TYPE_MISMATCH",
|
||||
"MARKUP_NOT_MEDIA",
|
||||
"NOT_MEDIA_PAYLOAD",
|
||||
"INVALID_VIDEO_METADATA",
|
||||
"VIDEO_SOURCE_UNRENDERABLE",
|
||||
"VIDEO_EXTRACTION_FAILED",
|
||||
|
||||
@@ -2,7 +2,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { copyFileSync, existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { MARKUP_NOT_MEDIA, MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import { NOT_MEDIA_PAYLOAD, NotMediaPayloadError } from "@hyperframes/engine";
|
||||
import {
|
||||
ASSET_MEDIA_TYPE_MISMATCH,
|
||||
AssetMediaTypeMismatchError,
|
||||
@@ -193,11 +193,11 @@ describe("preflightCompositionAssetMediaTypes", () => {
|
||||
});
|
||||
|
||||
// STUDIO-5433. This preflight sees every local media src regardless of its
|
||||
// authored timing, so it is the only place a markup payload behind a
|
||||
// authored timing, so it is the only place a document payload behind a
|
||||
// `data-end` video or a `loop`ing audio is caught before frames are captured
|
||||
// — for those elements the compiler never resolves a duration, so its own
|
||||
// sniff never runs.
|
||||
describe("markup payloads", () => {
|
||||
describe("non-media payloads", () => {
|
||||
beforeAll(() => {
|
||||
writeFileSync(
|
||||
join(projectDir, "streamed-preview.html"),
|
||||
@@ -216,9 +216,9 @@ describe("preflightCompositionAssetMediaTypes", () => {
|
||||
} catch (error) {
|
||||
caught = error;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(MarkupNotMediaError);
|
||||
expect(caught).toBeInstanceOf(NotMediaPayloadError);
|
||||
expect(caught).toMatchObject({
|
||||
code: MARKUP_NOT_MEDIA,
|
||||
code: NOT_MEDIA_PAYLOAD,
|
||||
owner: "user",
|
||||
retryable: false,
|
||||
});
|
||||
@@ -227,11 +227,11 @@ describe("preflightCompositionAssetMediaTypes", () => {
|
||||
expect(message).not.toContain(fixtureDir);
|
||||
});
|
||||
|
||||
it("reports markup ahead of the type mismatch the same file also produces", async () => {
|
||||
it("reports the document verdict ahead of the type mismatch the same file also produces", async () => {
|
||||
// An HTML page under a <video> is both "not media" and "not video". The
|
||||
// markup verdict is the actionable one; the mismatch is a symptom of it.
|
||||
// document verdict is the actionable one; the mismatch is a symptom of it.
|
||||
await expect(run({ videoSrc: "streamed-preview.html" })).rejects.toBeInstanceOf(
|
||||
MarkupNotMediaError,
|
||||
NotMediaPayloadError,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -244,7 +244,7 @@ describe("preflightCompositionAssetMediaTypes", () => {
|
||||
});
|
||||
|
||||
it("leaves an SVG image source alone", async () => {
|
||||
// ffprobe reads SVG through its svg_pipe demuxer, so markup is a
|
||||
// ffprobe reads SVG through its svg_pipe demuxer, so an XML body is a
|
||||
// legitimate <img> payload and must not be swept up by the sniff.
|
||||
await expect(run({ imageSrc: "brand-mark.svg" })).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import { existsSync } from "node:fs";
|
||||
import {
|
||||
fingerprintElementId,
|
||||
isMarkupPayload,
|
||||
MarkupNotMediaError,
|
||||
isNotMediaPayload,
|
||||
NotMediaPayloadError,
|
||||
probeMediaProfile,
|
||||
resolveProjectRelativeSrc,
|
||||
type AudioElement,
|
||||
@@ -131,13 +131,13 @@ export async function preflightCompositionAssetMediaTypes(input: {
|
||||
}
|
||||
|
||||
const mismatches: AssetMediaTypeMismatch[] = [];
|
||||
const markupFingerprints: string[] = [];
|
||||
const notMediaFingerprints: string[] = [];
|
||||
const entries = [...byPath];
|
||||
await Promise.all(
|
||||
entries.map(([resolvedPath, pathReferences]) =>
|
||||
withMediaProbeSlot(async () => {
|
||||
// STUDIO-5433. This preflight is the only place every local media src is
|
||||
// seen regardless of its authored timing, so it is where a markup
|
||||
// seen regardless of its authored timing, so it is where a document
|
||||
// payload behind a `data-end` video gets caught — the compiler's own
|
||||
// sniff only runs for elements whose duration it has to resolve.
|
||||
//
|
||||
@@ -146,9 +146,11 @@ export async function preflightCompositionAssetMediaTypes(input: {
|
||||
// source is non-fatal by existing policy (audioStage ships the render
|
||||
// without the track and reports `audioError`), so it is classified
|
||||
// per-element in audioMixer instead of aborted here.
|
||||
const markupCandidates = pathReferences.filter((ref) => ref.expected === "video");
|
||||
if (markupCandidates.length > 0 && (await isMarkupPayload(resolvedPath))) {
|
||||
markupFingerprints.push(...markupCandidates.map((ref) => fingerprintElementId(ref.id)));
|
||||
const sniffableReferences = pathReferences.filter((ref) => ref.expected === "video");
|
||||
if (sniffableReferences.length > 0 && (await isNotMediaPayload(resolvedPath))) {
|
||||
notMediaFingerprints.push(
|
||||
...sniffableReferences.map((ref) => fingerprintElementId(ref.id)),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -173,6 +175,6 @@ export async function preflightCompositionAssetMediaTypes(input: {
|
||||
|
||||
// Thrown ahead of the mismatch aggregate: "this file is an HTML page" is the
|
||||
// actionable diagnosis, while the type mismatch it also produces is a symptom.
|
||||
if (markupFingerprints.length > 0) throw new MarkupNotMediaError(markupFingerprints);
|
||||
if (notMediaFingerprints.length > 0) throw new NotMediaPayloadError(notMediaFingerprints);
|
||||
if (mismatches.length > 0) throw new AssetMediaTypeMismatchError(mismatches);
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ import { join } from "node:path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { interpolateVolumeGain } from "@hyperframes/core/media-volume-envelope";
|
||||
import { defaultLogger } from "../logger.js";
|
||||
import { MarkupNotMediaError } from "@hyperframes/engine";
|
||||
import { NotMediaPayloadError } from "@hyperframes/engine";
|
||||
import {
|
||||
collectExternalAssets,
|
||||
compileForRender,
|
||||
@@ -2278,12 +2278,12 @@ describe("sub-composition variable injection (render path, #2064)", () => {
|
||||
// from ffprobe — the `mov,mp4,…` prefix is ffprobe's demuxer probe order, NOT
|
||||
// the file's true format, so every alert routed as a codec/ffmpeg bug. The
|
||||
// byte-level sniff itself is unit-tested in
|
||||
// `engine/src/utils/markupPayload.test.ts`; what is pinned here is the
|
||||
// `engine/src/utils/notMediaPayload.test.ts`; what is pinned here is the
|
||||
// compiler's handling of the verdict, which differs by element type.
|
||||
|
||||
describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
describe("compileForRender non-media payload sniff (STUDIO-5433)", () => {
|
||||
function writeProject(mediaTag: string): string {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-markup-sniff-e2e-"));
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-payload-sniff-e2e-"));
|
||||
mkdirSync(join(projectDir, "assets"));
|
||||
writeFileSync(
|
||||
join(projectDir, "assets", "nested.html"),
|
||||
@@ -2307,7 +2307,7 @@ describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
return projectDir;
|
||||
}
|
||||
|
||||
it("aborts with MarkupNotMediaError before ffprobe when a <video> src is an HTML payload", async () => {
|
||||
it("aborts with NotMediaPayloadError before ffprobe when a <video> src is an HTML payload", async () => {
|
||||
// Mimics STUDIO-5433: an a-roll element whose src points at a legitimate
|
||||
// 6.5 KB `<!DOCTYPE html>` preview page instead of the rendered MP4.
|
||||
const projectDir = writeProject(
|
||||
@@ -2320,11 +2320,11 @@ describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(MarkupNotMediaError);
|
||||
const err = caught as MarkupNotMediaError;
|
||||
expect(caught).toBeInstanceOf(NotMediaPayloadError);
|
||||
const err = caught as NotMediaPayloadError;
|
||||
// Routing metadata, not just a readable string: these are what the server's
|
||||
// SAFE_RENDER_ERROR_CODES allowlist and the distributed retry sets key off.
|
||||
expect(err.code).toBe("MARKUP_NOT_MEDIA");
|
||||
expect(err.code).toBe("NOT_MEDIA_PAYLOAD");
|
||||
expect(err.owner).toBe("user");
|
||||
expect(err.retryable).toBe(false);
|
||||
// Correlation is the hashed element id — the authored src never reaches a
|
||||
@@ -2336,7 +2336,7 @@ describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
// error and this assertion would fail.
|
||||
});
|
||||
|
||||
it("drops an <audio> markup payload to duration 0 and warns instead of failing the render", async () => {
|
||||
it("drops an <audio> document payload to duration 0 and warns instead of failing the render", async () => {
|
||||
// The audio/video split is the compiler's contract: an unprobeable audio
|
||||
// src is excluded from the render, and only video surfaces its probe
|
||||
// failure. A hard abort here would take down renders that used to succeed
|
||||
@@ -2355,7 +2355,7 @@ describe("compileForRender markup sniff (STUDIO-5433)", () => {
|
||||
);
|
||||
|
||||
expect(compiled.html).not.toContain('id="a1" src="assets/nested.html" data-end');
|
||||
expect(warnings.join("\n")).toContain("HTML/XML document");
|
||||
expect(warnings.join("\n")).toContain("text document");
|
||||
expect(warnings.join("\n")).toContain("a1");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,8 +51,8 @@ import {
|
||||
type AudioVolumeKeyframe,
|
||||
type MediaProbeProfile,
|
||||
analyzeKeyframeIntervals,
|
||||
assertNotMarkupPayload,
|
||||
MarkupNotMediaError,
|
||||
assertMediaPayload,
|
||||
NotMediaPayloadError,
|
||||
probeMediaProfile,
|
||||
} from "@hyperframes/engine";
|
||||
import { assertPublicHttpsUrl, downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
|
||||
@@ -439,25 +439,25 @@ async function resolveMediaDuration(
|
||||
return withMediaProbeSlot(async () => {
|
||||
let profile: MediaProbeProfile;
|
||||
try {
|
||||
// Markup sniff (STUDIO-5433): if an authoring bug hands us an HTML/XML
|
||||
// Payload sniff (STUDIO-5433): if an authoring bug hands us a text
|
||||
// payload (e.g. an unresolved nested-composition preview URL), fail with
|
||||
// a typed MarkupNotMediaError instead of letting ffprobe emit an opaque
|
||||
// a typed NotMediaPayloadError instead of letting ffprobe emit an opaque
|
||||
// `[mov,mp4,...] moov atom not found` that routes as a codec bug.
|
||||
// Deliberately inside this try: the audio/video split below is the
|
||||
// contract, so a bad audio src must still degrade to duration 0 rather
|
||||
// than take down the whole render.
|
||||
await assertNotMarkupPayload(filePath, elementIdentity);
|
||||
await assertMediaPayload(filePath, elementIdentity);
|
||||
profile = await probeMediaProfile(filePath);
|
||||
} catch (error) {
|
||||
// Preserve the historical split: invalid video sources surface their
|
||||
// probe failure, while invalid/unreadable audio sources resolve to zero
|
||||
// duration and are excluded by the compiler.
|
||||
if (tagName !== "video") {
|
||||
if (error instanceof MarkupNotMediaError) {
|
||||
if (error instanceof NotMediaPayloadError) {
|
||||
// Dropping it silently is what let STUDIO-5433 resurface downstream
|
||||
// as `prepare/ffmpeg_failed` with owner "system".
|
||||
log?.warn(
|
||||
`[compile] Audio "${elementIdentity}" (${src}) is an HTML/XML document, not a media ` +
|
||||
`[compile] Audio "${elementIdentity}" (${src}) is a text document, not a media ` +
|
||||
"file — the element is dropped from the render. Point it at a rendered media file.",
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user