diff --git a/examples/aws-lambda/template.yaml b/examples/aws-lambda/template.yaml index fc28dc1f0b2..0a1fd60758f 100644 --- a/examples/aws-lambda/template.yaml +++ b/examples/aws-lambda/template.yaml @@ -460,6 +460,7 @@ Resources: - PlanV2IntegrityError - PLAN_ARTIFACT_DIGEST_MISMATCH - ChromeBinaryUnavailableError + - ImageMissingFromFrameError MaxAttempts: 0 - ErrorEquals: [Lambda.TooManyRequestsException] IntervalSeconds: 5 diff --git a/packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts b/packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts index d8d69359dcc..5ff26815a31 100644 --- a/packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts +++ b/packages/aws-lambda/src/cdk/HyperframesRenderStack.snapshot.test.ts @@ -234,6 +234,14 @@ describe("HyperframesRenderStack — snapshot", () => { } }); + it("stops a chunk's retries for an image a frame shows but never got, in SAM and CDK", () => { + for (const definition of [SYNTHED.definition, readSamDefinition()]) { + const errors = new Set(); + collectNonRetryableErrors(getV2TaskStates(definition).RenderChunkV2, errors); + expect(errors.has("ImageMissingFromFrameError")).toBe(true); + } + }); + it("lists the error the SDK checks for before rendering in every SAM and CDK plan task", () => { for (const definition of [SYNTHED.definition, readSamDefinition()]) { const errors = new Set(); diff --git a/packages/aws-lambda/src/cdk/HyperframesRenderStack.ts b/packages/aws-lambda/src/cdk/HyperframesRenderStack.ts index 48ca06a2f35..ed0d6fc65d6 100644 --- a/packages/aws-lambda/src/cdk/HyperframesRenderStack.ts +++ b/packages/aws-lambda/src/cdk/HyperframesRenderStack.ts @@ -250,6 +250,7 @@ export class HyperframesRenderStack extends Construct { "PlanV2IntegrityError", "PLAN_ARTIFACT_DIGEST_MISMATCH", "ChromeBinaryUnavailableError", + "ImageMissingFromFrameError", ]; const NON_RETRYABLE_ASSEMBLE = [ "FFMPEG_VERSION_MISMATCH", diff --git a/packages/cli/src/ui/renderDiagnostic.test.ts b/packages/cli/src/ui/renderDiagnostic.test.ts index f20b24df91d..deae15433bd 100644 --- a/packages/cli/src/ui/renderDiagnostic.test.ts +++ b/packages/cli/src/ui/renderDiagnostic.test.ts @@ -133,6 +133,13 @@ describe("render diagnostic wire", () => { }), "COMPOSITION_SETUP_TIMEOUT", ], + [ + new CaptureFailure({ + kind: "authoring", + message: `Frame 0 shows ${PRIVATE}, which did not load in time, so the render stopped rather than make a video missing it.`, + }), + "IMAGE_MISSING_FROM_FRAME", + ], [ new CaptureFailure({ kind: "authoring", message: PRIVATE, pageErrorLines: [PRIVATE] }), "PAGE_ERROR", diff --git a/packages/cli/src/ui/renderDiagnostic.ts b/packages/cli/src/ui/renderDiagnostic.ts index 27947589dce..0bc610c9459 100644 --- a/packages/cli/src/ui/renderDiagnostic.ts +++ b/packages/cli/src/ui/renderDiagnostic.ts @@ -42,6 +42,8 @@ function producerCodeOf(error: unknown): string | undefined { function captureCodeOf(error: unknown, message: string): string | undefined { if (/Composition has zero duration/.test(message)) return "COMPOSITION_ZERO_DURATION"; if (/Composition setup did not finish/.test(message)) return "COMPOSITION_SETUP_TIMEOUT"; + if (/so the render stopped rather than make a video missing it/.test(message)) + return "IMAGE_MISSING_FROM_FRAME"; if (!(error instanceof CaptureFailure)) return undefined; return error.pageErrorLines.length > 0 ? "PAGE_ERROR" : `CAPTURE_${error.kind.toUpperCase()}`; } diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 0d315fce121..315c5b3508f 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -155,6 +155,7 @@ export { type DiscardWarmupInnerCapture, type StaticVerificationOutcome, VfxFailureError, + ImageMissingFromFrameError, waitForImagesReady, evaluateHfDiagnostic, buildZeroDurationDiagnostic, diff --git a/packages/engine/src/services/frameCapture-lateImages.test.ts b/packages/engine/src/services/frameCapture-lateImages.test.ts new file mode 100644 index 00000000000..cf605539e53 --- /dev/null +++ b/packages/engine/src/services/frameCapture-lateImages.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Page } from "puppeteer-core"; +import { + ImageMissingFromFrameError, + waitForShownLateImages, + type CaptureSession, +} from "./frameCapture.js"; + +type Reading = { arrived: string[]; waiting: string[]; broken: string[] }; + +function sessionReading(readings: Reading[], late: string[], playerReadyTimeout = 60_000) { + const evaluate = vi.fn(async () => readings.shift() ?? { arrived: [], waiting: [], broken: [] }); + const session = { + page: { evaluate } as unknown as Page, + lateImages: new Set(late), + warnings: [], + config: { playerReadyTimeout }, + } as unknown as CaptureSession; + return { session, evaluate }; +} + +const PHOTO = "http://localhost:1/assets/photo.png"; + +describe("waitForShownLateImages", () => { + it("never touches the page when no image was late", async () => { + const { session, evaluate } = sessionReading([], []); + await waitForShownLateImages(session, 0); + expect(evaluate).not.toHaveBeenCalled(); + }); + + it("waits for a late image the frame shows, then stops asking about it", async () => { + const { session, evaluate } = sessionReading( + [ + { arrived: [], waiting: [PHOTO], broken: [] }, + { arrived: [PHOTO], waiting: [], broken: [] }, + ], + [PHOTO], + ); + await waitForShownLateImages(session, 12); + expect(evaluate).toHaveBeenCalledTimes(2); + expect(session.lateImages.size).toBe(0); + await waitForShownLateImages(session, 13); + expect(evaluate).toHaveBeenCalledTimes(2); + }); + + it("captures on when the late image is not in this frame, and keeps it listed", async () => { + const { session } = sessionReading([{ arrived: [], waiting: [], broken: [] }], [PHOTO]); + await waitForShownLateImages(session, 0); + expect([...session.lateImages]).toEqual([PHOTO]); + }); + + // fails without the fix: the frame was captured with the image missing and the render reported success. + it("stops the render when a shown image has not arrived by the limit, naming frame and path", async () => { + const { session } = sessionReading([{ arrived: [], waiting: [PHOTO], broken: [] }], [PHOTO], 0); + const stopped = await waitForShownLateImages(session, 360).catch((error: unknown) => error); + expect(stopped).toBeInstanceOf(ImageMissingFromFrameError); + expect(stopped).toMatchObject({ frameIndex: 360, sources: [PHOTO], failedToLoad: false }); + expect((stopped as Error).message).toContain("did not load in time"); + expect(session.warnings).toEqual([ + expect.objectContaining({ + code: "image_missing_from_frame", + message: (stopped as Error).message, + }), + ]); + }); + + it("stops at once for a shown image that failed to load, since waiting cannot help", async () => { + const { session, evaluate } = sessionReading( + [{ arrived: [], waiting: [], broken: [PHOTO] }], + [PHOTO], + ); + const stopped = await waitForShownLateImages(session, 5).catch((error: unknown) => error); + expect(stopped).toBeInstanceOf(ImageMissingFromFrameError); + expect((stopped as Error).message).toContain("failed to load"); + expect(evaluate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index a876cd93acc..184c34c6743 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -153,7 +153,8 @@ type CaptureStep = | "waiting for page seek handlers" | "loading color-grading LUTs" | "compositing the page" - | "capturing the frame"; + | "capturing the frame" + | "waiting for a late image"; export interface CaptureSession { browser: Browser; @@ -246,6 +247,8 @@ export interface CaptureSession { subTimelineWaitMemo: SubTimelineWaitMemo; /** Structured readiness warnings surfaced to the producer's render policy. */ warnings: CaptureWarning[]; + /** Images still loading when capture began: a frame that shows one waits for it (waitForShownLateImages). */ + lateImages?: Set; initTelemetry?: { initDurationMs: number; tweenCount: number; @@ -2519,6 +2522,99 @@ function recordCaptureWarnings(session: CaptureSession, warnings: readonly Captu } } +function recordReadiness(session: CaptureSession, warnings: readonly CaptureWarning[]): void { + recordCaptureWarnings(session, warnings); + for (const warning of warnings) { + if (warning.code !== "media_readiness_timeout" || warning.details?.mediaType !== "image") + continue; + session.lateImages ??= new Set(); + for (const source of warning.details.sources ?? []) session.lateImages.add(source); + } +} + +/** Thrown when a frame would show an image that never arrived: the video would silently lack it. */ +export class ImageMissingFromFrameError extends Error { + constructor( + readonly frameIndex: number, + readonly sources: readonly string[], + readonly failedToLoad: boolean, + ) { + super( + `Frame ${frameIndex} shows ${sources.join(", ")}, which ${failedToLoad ? "failed to load" : "did not load in time"}, ` + + `so the render stopped rather than make a video missing it. ` + + (failedToLoad + ? `Check that the image exists and can be opened.` + : `Try fewer workers (--workers 1) or a longer --player-ready-timeout.`), + ); + this.name = "ImageMissingFromFrameError"; + } +} + +const LATE_IMAGE_POLL_MS = 100; + +/** Before a frame: wait for any late image it shows, for as long as the page was given to get ready, else stop. */ +export async function waitForShownLateImages( + session: CaptureSession, + frameIndex: number, +): Promise { + const lateImages = session.lateImages; + if (!lateImages?.size) return; + session.captureStep = "waiting for a late image"; + const timeoutMs = session.config?.playerReadyTimeout ?? DEFAULT_CONFIG.playerReadyTimeout; + const deadline = Date.now() + timeoutMs; + for (;;) { + const { arrived, waiting, broken } = await session.page.evaluate(async (urls: string[]) => { + const listed = new Set(urls); + const found = Array.from(document.querySelectorAll("img")).filter((img) => + listed.has(img.src), + ); + const loaded = found.filter((img) => img.complete && img.naturalWidth > 0); + await Promise.allSettled(loaded.map((img) => img.decode())); + const inViewport = (img: HTMLImageElement) => { + const box = img.getBoundingClientRect(); + return ( + box.width > 0 && + box.height > 0 && + box.right > 0 && + box.bottom > 0 && + box.left < window.innerWidth && + box.top < window.innerHeight + ); + }; + const shown = found.filter( + (img) => + !loaded.includes(img) && + img.checkVisibility({ opacityProperty: true, visibilityProperty: true }) && + inViewport(img), + ); + return { + arrived: loaded.map((img) => img.src), + waiting: shown.filter((img) => !img.complete).map((img) => img.src), + broken: shown.filter((img) => img.complete).map((img) => img.src), + }; + }, Array.from(lateImages)); + for (const source of arrived) lateImages.delete(source); + const missing = Date.now() >= deadline ? [...waiting, ...broken] : broken; + if (missing.length > 0) { + const stopped = new ImageMissingFromFrameError( + frameIndex, + [...new Set(missing.map(sanitizeDiagnosticUrl))].sort(), + broken.length > 0, + ); + recordCaptureWarnings(session, [ + { + code: "image_missing_from_frame", + message: stopped.message, + details: { mediaType: "image", sources: [...stopped.sources] }, + }, + ]); + throw stopped; + } + if (waiting.length === 0) return; + await new Promise((resolve) => setTimeout(resolve, LATE_IMAGE_POLL_MS)); + } +} + /** Report background urls that timed out decoding; the shown url drops credentials and query. */ export function recordBackgroundDecodeTimeouts( session: CaptureSession, @@ -3109,7 +3205,7 @@ export async function initializeSession(session: CaptureSession): Promise `Continuing render. Affected videos will appear as blank/black frames.`, ); } - recordCaptureWarnings( + recordReadiness( session, await collectMediaReadinessWarnings(page, skipVideoIds, pageReadyTimeout), ); @@ -3251,7 +3347,7 @@ export async function initializeSession(session: CaptureSession): Promise `Continuing render. Affected videos will appear as blank/black frames.`, ); } - recordCaptureWarnings( + recordReadiness( session, await collectMediaReadinessWarnings(page, bfSkipVideoIds, pageReadyTimeout), ); @@ -3426,6 +3522,7 @@ async function prepareFrameForCapture( const seekStart = Date.now(); session.captureStep = "seeking the timeline"; const hasPendingComposite = await seekPageTimeline(page, quantizedTime, seekOptions); + await waitForShownLateImages(session, frameIndex); session.captureStep = "decoding CSS background images"; // Mocked pages answer evaluate() with undefined, so tolerate a missing list. @@ -4860,6 +4957,7 @@ async function captureAccumulatedFrame( const eventfulSeekStart = Date.now(); session.captureStep = "seeking the timeline"; await seekPageTimeline(session.page, frameTime, undefined); + await waitForShownLateImages(session, frameIndex); const totals = { seekMs: Date.now() - eventfulSeekStart, beforeCaptureMs: 0, screenshotMs: 0 }; if (session.onBeforeCapture) { // Sub-samples leave a video's style alone: inject at frameTime first so the held frame @@ -5283,6 +5381,19 @@ export async function captureFramesBatchPipelined( } assertVfxIntact(session); for (const frameIndex of frameIndices) assertNotRiskFrame(session, frameIndex); + // The fused batch seeks inside the page, with no stop between frames to wait for a late image. + if (session.lateImages?.size) { + const oneByOne: Array<{ frameIndex: number; encodeResult: Promise }> = []; + for (const [i, frameIndex] of frameIndices.entries()) { + const { encodeResult } = await captureFrameToBufferPipelined( + session, + frameIndex, + times[i] ?? 0, + ); + oneByOne.push({ frameIndex, encodeResult }); + } + return oneByOne; + } const startTime = Date.now(); const fps = fpsToNumber(options.fps); const quantized = times.map((t) => quantizeTimeToFrame(t, fps)); diff --git a/packages/engine/src/types.ts b/packages/engine/src/types.ts index e8113395610..145cb0d5c4b 100644 --- a/packages/engine/src/types.ts +++ b/packages/engine/src/types.ts @@ -41,6 +41,7 @@ export interface SubTimelineWaitMemo { export type CaptureWarningCode = | "media_readiness_timeout" | "media_load_failed" + | "image_missing_from_frame" | "audio_processing_failed" | "sub_timeline_readiness_timeout" | "sub_timeline_script_failure" diff --git a/packages/gcp-cloud-run/src/server.test.ts b/packages/gcp-cloud-run/src/server.test.ts index 0e21158644a..3eb2cd0c5ad 100644 --- a/packages/gcp-cloud-run/src/server.test.ts +++ b/packages/gcp-cloud-run/src/server.test.ts @@ -429,6 +429,41 @@ describe("createApp HTTP mapping", () => { expect(await res.json()).toMatchObject({ error: "PLAN_PROTOCOL_UNSUPPORTED" }); }); + it("does not retry a chunk whose frame shows an image that never arrived", async () => { + const gcs = new FakeGcs(); + await seedProjectTar(gcs, planBody.ProjectGcsUri); + const root = mkTmp("hf-image-missing-"); + const deps = depsWith(gcs, { + planV2WithPublisher: async (_projectDir, _config, publisher) => { + const v1Dir = join(root, "v1"); + makeMinimalV1PlanDir(v1Dir, false); + return publishPlanV2FromExecutionPlan(v1Dir, publisher); + }, + renderChunk: async () => { + throw Object.assign(new Error("test"), { name: "ImageMissingFromFrameError" }); + }, + }); + const planned = await dispatch(planBody, deps); + if (!("PlanProtocol" in planned) || planned.PlanProtocol !== "v2") { + throw new Error("expected v2 plan result"); + } + const res = await createApp(deps).request("/", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + Action: "renderChunk", + PlanV2ManifestGcsUri: planned.PlanV2ManifestGcsUri, + PlanV2ArtifactGcsPrefix: planned.PlanV2ArtifactGcsPrefix, + PlanHash: planned.PlanHash, + ChunkIndex: 0, + ChunkOutputGcsPrefix: "gs://b/renders/image-missing/", + Format: "mp4", + }), + }); + expect(res.status).toBe(400); + expect(await res.json()).toMatchObject({ error: "ImageMissingFromFrameError" }); + }); + it.each([ ["INVALID_VIDEO_METADATA", 400], [PLAN_V2_INTEGRITY_UNRECOVERABLE, 400], diff --git a/packages/gcp-cloud-run/src/server.ts b/packages/gcp-cloud-run/src/server.ts index 1eae3b220db..003e896b9de 100644 --- a/packages/gcp-cloud-run/src/server.ts +++ b/packages/gcp-cloud-run/src/server.ts @@ -683,6 +683,7 @@ const NON_RETRYABLE_ERROR_NAMES = new Set([ "BROWSER_GPU_NOT_SOFTWARE", "FONT_FETCH_FAILED", "ChromeBinaryUnavailableError", + "ImageMissingFromFrameError", ]); /** diff --git a/packages/producer/src/services/distributed/renderChunk.ts b/packages/producer/src/services/distributed/renderChunk.ts index 83c17fb451a..ad556e1a7d0 100644 --- a/packages/producer/src/services/distributed/renderChunk.ts +++ b/packages/producer/src/services/distributed/renderChunk.ts @@ -383,12 +383,24 @@ export async function runCaptureWithScreenshotFallback(input: { try { return await input.run(input.forceScreenshot); } catch (error) { - if (input.forceScreenshot || (await input.canRetry?.()) === false) throw error; - if (!shouldRetryChunkCaptureWithScreenshot(error)) throw error; + if (input.forceScreenshot || (await input.canRetry?.()) === false) + throw imageMissingIn(error) ?? error; + if (!shouldRetryChunkCaptureWithScreenshot(error)) throw imageMissingIn(error) ?? error; input.onFallback?.(error); await input.resetForScreenshotRetry(); - return await input.run(true); + return await input.run(true).catch((retryError: unknown) => { + throw imageMissingIn(retryError) ?? retryError; + }); + } +} + +function imageMissingIn(error: unknown): unknown { + let e = error; + for (let depth = 0; depth < 5 && typeof e === "object" && e !== null; depth++) { + if ((e as { name?: unknown }).name === "ImageMissingFromFrameError") return e; + e = (e as { cause?: unknown }).cause; } + return undefined; } /** diff --git a/packages/producer/src/services/distributed/renderChunkFallback.test.ts b/packages/producer/src/services/distributed/renderChunkFallback.test.ts index 09b631f2859..5a8956996e9 100644 --- a/packages/producer/src/services/distributed/renderChunkFallback.test.ts +++ b/packages/producer/src/services/distributed/renderChunkFallback.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, expect, it } from "bun:test"; import { + CaptureFailure, type CaptureSession, + ImageMissingFromFrameError, createFrameLookupTable, readWebGlVendorInfoFromCanvas, resolveConfig, @@ -15,6 +17,7 @@ import { runCaptureWithScreenshotFallback, shouldRetryChunkCaptureWithScreenshot, } from "./renderChunk.js"; +import { CaptureStageError } from "../render/captureStageError.js"; const originalProbeTimeout = process.env.PRODUCER_BEGINFRAME_PROBE_TIMEOUT_MS; @@ -306,3 +309,48 @@ describe("a chunk whose workers launched in different capture modes", () => { expect(() => observedChunkCaptureMode([{ captureMode: "gpu" }])).toThrow(/invalid mode/); }); }); + +describe("a chunk whose frame shows an image that never arrived", () => { + const stopped = () => new ImageMissingFromFrameError(0, ["photo.png"], false); + const throughMultiWorkerCapture = () => + new CaptureStageError({ + cause: new CaptureFailure({ + kind: "authoring", + message: "[Parallel] Capture failed: Worker 0", + cause: new CaptureFailure({ kind: "authoring", message: "worker", cause: stopped() }), + }), + browserConsole: [], + }); + const run = (first: unknown, second?: unknown) => + runCaptureWithScreenshotFallback({ + forceScreenshot: false, + run: async (forceScreenshot) => { + throw forceScreenshot ? second : first; + }, + resetForScreenshotRetry: () => {}, + }); + + it("throws the image's own error from a multi-worker capture, so cloud retry lists see its name", async () => { + await expect(run(throughMultiWorkerCapture())).rejects.toMatchObject({ + name: "ImageMissingFromFrameError", + sources: ["photo.png"], + }); + }); + + it("throws it from a single-worker capture too", async () => { + const wrapped = new CaptureStageError({ cause: stopped(), browserConsole: [] }); + await expect(run(wrapped)).rejects.toMatchObject({ name: "ImageMissingFromFrameError" }); + }); + + it("throws it when it comes from the screenshot retry", async () => { + const beginFrameFailed = new Error("HeadlessExperimental.beginFrame timed out"); + await expect(run(beginFrameFailed, throughMultiWorkerCapture())).rejects.toMatchObject({ + name: "ImageMissingFromFrameError", + }); + }); + + it("leaves any other capture failure as it was", async () => { + const other = new CaptureStageError({ cause: new Error("boom"), browserConsole: [] }); + await expect(run(other)).rejects.toBe(other); + }); +});