Mark items failed only once no retry is left

Merged
FR
2h ago
+374-223

Stacked on #705 (fredrivett/single-cover-enrichment); review that first. This PR's own diff is the second commit.

What & why

Six item-processing tasks (classify-url, analyze-image, enrich-item, analyze-media-cover, analyze-document, import-pdf) set processingStatus: "failed" in their catch and rethrew so Trigger.dev would retry, which was the documented convention. With retry: { maxAttempts: 2 } that meant:

  • A first-attempt failure showed a red "Failed" card while Trigger's retry was still queued, and since the grid only polls processing items, a successful retry stayed hidden until reload.
  • Clicking Retry in that window passed claimFailedRetry and started a second paid run beside Trigger's own retry, both writing the same item.
  • Failures no retry can fix (a 403/404 from the source, unusable content) were fetched and reported twice.

Now each of those tasks spreads itemFailureHooks (app/trigger/item-failure.ts):

  • onFailure, which Trigger calls only once no attempt is left, reports the error and marks the item failed. Until then the item stays processing, so the grid keeps polling and Retry stays closed. A completed item is always kept as it is: user-initiated runs (Retry, reassign) set the item to processing before they enqueue, so a completed item means a later stage or a background re-run (admin reprocess, cover swap) failed, and its earlier result stands.
  • analyze-image keeps #705's fallback: when the run carried a book or product's text, onFailure reports and enriches from that text instead of marking the item failed.
  • catchError returns skipRetrying for failures isPermanentFailure recognises: any SafeFetchError (SSRF gate, invalid or too many redirects, oversized body), plus source_blocked (401/403/429), source_not_found, unsupported_content, file_unreadable, document_too_long. Trigger still passes the original error to onFailure, so the stored reason is unchanged. This replaces import-pdf's AbortTaskRunError.
  • analyze-document still drops a run whose PDF was replaced mid-analysis; import-pdf fires pdf_import_failed from onFailure, so once per import rather than once per attempt.
  • The convention in app/trigger/AGENTS.md and the add-trigger-task skill describe the new pattern.

Runs that never reach onFailure

Per the Trigger.dev v4 docs, onFailure isn't called for a run stopped at maxDuration, crashed, cancelled, or expired in the queue (TTL); an out-of-memory kill ends the process, so no hook runs in that attempt. The previous catch didn't run in those cases either, so the stuck-items reaper (reap-stuck-items) was already what marked them stalled, and still is. Items now stay processing through the retry gap (seconds), each attempt advances processingStartedAt, and the 4h threshold stays well above the 2h TTL plus the longest maxDuration.

Checklist

  • User-facing change → added/updated a PostHog event (and a funnel/insight for a new flow), or N/A — no new flow; pdf_import_failed now fires once per failed import (after the last attempt) instead of once per attempt
  • User-facing or service change → docs updated (README features/roadmap/services, .env.example, in-app help, CONTRIBUTING.md, onboarding checklist), or N/A — app/trigger/AGENTS.md convention and the add-trigger-task skill updated; nothing in the user docs changes
  • New error paths report via captureServerException / the error boundary, or N/A — failures report via captureServerException from onFailure, with { task, itemId } plus the payload's url / fileKey when present
  • New behavior is covered by tests, or N/A — unit tests for isPermanentFailure, classify-url, analyze-document and import-pdf; real-Postgres integration tests for analyze-image and the PDF import (an attempt leaves the item processing; the final failure marks it failed with its reason; a completed item is never regressed; permanent failures skip the retry)
  • User-facing change → demo video attached (record-pr-demo skill); refactor through a user flow → parity run, or N/A — N/A: retries happen only in Trigger.dev, which isn't configured in the demo stack, so the "Failed during a pending retry" state can't be reproduced there
  • If this fixes a recurring defect, considered a guardrail via the learn skill, or N/A — the convention itself was the cause; it's replaced in trigger/AGENTS.md and the skill template, with one shared hook so new item tasks get it by spreading it

Notes

  • Verified: the new tests fail on the previous code and pass now; bun run check:fix clean; the full unit suite and the trigger/ integration tests pass. I read SDK 4.5.16's task executor to confirm onFailure runs only when the attempt isn't retried, and receives the original error when catchError skips retrying.
  • Not verified: nothing ran through a real Trigger.dev deployment (not available locally), so retries, catchError and onFailure were exercised by calling the hooks directly, not end to end.
  • A 429 is in source_blocked and so is no longer retried; Trigger's retry comes seconds later, well inside any rate-limit window.

Summary by cubic

Six item-processing tasks (classify-url, analyze-image, enrich-item, analyze-media-cover, analyze-document, import-pdf) now mark an item failed only after Trigger.dev's last retry attempt, instead of on the first catch. Marking early showed a red "Failed" card while a retry was queued, hid that retry's success from the grid poll (which only follows processing items), and let Retry start a second paid run beside Trigger's own.

  • Tasks spread the shared itemFailureHooks (app/trigger/item-failure.ts): onFailure reports (with the payload's url/fileKey) and marks the item failed once no attempt is left, keeping a completed item as it is; catchError skips retrying failures that can't succeed, treating every SafeFetchError (SSRF gate, redirect cap, oversized body) as permanent alongside a 403/404 source or unusable content.
  • analyze-image still enriches from the item's text instead of marking it failed when the run carried book/product text, so the item completes via its text.
  • import-pdf's pdf_import_failed event fires once per failed import from onFailure, and its AbortTaskRunError shortcut was replaced by catchError.
  • Hooks don't run for runs stopped at maxDuration, crashed, cancelled or expired in the queue; the stuck-items reaper still marks those items stalled.
  • The error-handling convention is documented in app/trigger/AGENTS.md and the add-trigger-task skill.