From 11c22d78b5064a62b3c796afa2b60676e7c1c614 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Thu, 24 Sep 2026 16:37:16 +0000 Subject: [PATCH] Retry a failed render instead of returning the failure forever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dedupe is keyed by a hash of the design, and the reuse branch handed back whatever row it found — including a failed one. So the first failure became a permanent verdict on that copy: re-saving the campaign produced the same hash, found the same failed row, and queued nothing. The render card's advice to edit and save to try again was advice that could not work, because an unedited campaign hashes identically. This surfaced immediately after the animated banner bug: three good pre-rolls had been failed by a broken GIF encode, and with that fixed and deployed the campaigns still would not render, because the failed rows were being handed straight back. A failed row is now requeued and re-enqueued, capped at three attempts. The cap matters because the failure may be deterministic — a snapshot this renderer cannot draw — and the row is keyed by design rather than by attempt, so without one every save of an unrenderable campaign would queue the same doomed work again. Co-Authored-By: Claude Opus 5 (1M context) --- lib/ads/video/jobs.ts | 48 +++++++++++++++++++++++++++++++++++- tests/ads-video-jobs.test.ts | 34 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/ads/video/jobs.ts b/lib/ads/video/jobs.ts index a9eac8b..51fd77e 100644 --- a/lib/ads/video/jobs.ts +++ b/lib/ads/video/jobs.ts @@ -29,6 +29,15 @@ export type RenderState = "queued" | "rendering" | "validating" | "ready" | "fai * unaffected and the ones that aren't lose a trailing clause rather than half * a word. */ +/** + * How many times one design may be re-attempted. + * + * A render can fail for a reason that will never change, and the row is keyed + * by the design rather than by the attempt, so without a cap every save of an + * unrenderable campaign would queue the same doomed work again. + */ +export const MAX_RENDER_ATTEMPTS = 3; + export function trimHeadlineForVideo(headline: string): string { const words = headline.trim().split(/\s+/).filter(Boolean); if (words.length <= MAX_HEADLINE_WORDS) return words.join(" "); @@ -142,12 +151,49 @@ export async function ensureRenderJob( const existing = await supabase .from("ad_video_jobs") - .select("id, state, revision") + .select("id, state, revision, attempts") .eq("render_hash", hash) .eq("output_profile", DEFAULT_OUTPUT_PROFILE) .maybeSingle(); if (existing.data) { + // A failed job must not become a permanent verdict on a design. Dedupe + // handed the failed row straight back, so once a render failed, that exact + // copy could never render again: re-saving hit the same hash, and the card + // telling the advertiser to edit and retry was advice that could not work. + // + // Capped, because the failure may be deterministic — a snapshot this + // renderer simply cannot draw — and an uncapped retry would re-run it on + // every save forever. + const state = existing.data.state as RenderState; + const attempts = Number(existing.data.attempts ?? 0); + if (state === "failed" && attempts < MAX_RENDER_ATTEMPTS) { + await supabase + .from("ad_video_jobs") + .update({ state: "queued", error_code: null }) + .eq("id", existing.data.id as string); + const requeued = await enqueueRender({ + renderHash: hash, + profile: DEFAULT_OUTPUT_PROFILE, + data: { + jobRowId: existing.data.id as string, + ownerId: args.ownerId, + campaignId: args.campaignId ?? null, + creativeId: args.creativeId ?? null, + revision: existing.data.revision as number, + snapshot: args.snapshot, + profile: DEFAULT_OUTPUT_PROFILE, + audioSlotSupported: false, + }, + }).catch(() => false); + return { + jobId: existing.data.id as string, + state: "queued" as RenderState, + revision: existing.data.revision as number, + reused: true, + enqueued: requeued, + }; + } return { jobId: existing.data.id as string, state: existing.data.state as RenderState, diff --git a/tests/ads-video-jobs.test.ts b/tests/ads-video-jobs.test.ts index 2aca088..ca28865 100644 --- a/tests/ads-video-jobs.test.ts +++ b/tests/ads-video-jobs.test.ts @@ -4,6 +4,7 @@ import { downloadableAsset, ensureRenderJob, ensureVideoCreative, + MAX_RENDER_ATTEMPTS, renderStateLabel, snapshotFromCreatives, streamingReady, @@ -171,6 +172,39 @@ describe("render jobs dedupe on the design, not the campaign", () => { expect(db.inserts).toHaveLength(0); }); + it("retries a failed job instead of handing the failure back forever", async () => { + // Dedupe is keyed by the design, so returning the failed row made a failure + // permanent for that copy: re-saving hit the same hash, and the card's + // advice to edit and retry could not work. + const db = fakeDb({ existingJob: { id: "job-f", state: "failed", revision: 1, attempts: 1 } }); + const res = await ensureRenderJob(db.client as never, { + ownerId: "o", + campaignId: "c", + creativeId: "cr", + snapshot, + revision: 1, + }); + expect(res).toMatchObject({ jobId: "job-f", state: "queued", reused: true }); + expect(db.updates[0].patch).toMatchObject({ state: "queued", error_code: null }); + }); + + it("stops retrying once the cap is reached", async () => { + // The failure may be deterministic, and the row is keyed by design rather + // than by attempt, so an uncapped retry re-runs doomed work on every save. + const db = fakeDb({ + existingJob: { id: "job-f", state: "failed", revision: 1, attempts: MAX_RENDER_ATTEMPTS }, + }); + const res = await ensureRenderJob(db.client as never, { + ownerId: "o", + campaignId: "c", + creativeId: "cr", + snapshot, + revision: 1, + }); + expect(res).toMatchObject({ state: "failed", reused: true }); + expect(db.updates).toHaveLength(0); + }); + it("writes the row before trying to enqueue", async () => { const db = fakeDb({ existingJob: null }); const res = await ensureRenderJob(db.client as never, {