fix: batch trigger returns stale failed runs when idempotency key points to a dead run - #4818
Conversation
BatchTriggerV3Service.#prepareRunData only checked time-based expiration on cached idempotency key lookups, but never checked whether the matched run was in a terminal failure state. The single-trigger path (IdempotencyKeyConcern.handleExistingRun) correctly calls shouldIdempotencyKeyBeCleared(status) and re-triggers in that case. This meant batchTrigger with an idempotency key that pointed at a CRASHED, SYSTEM_FAILURE, TIMED_OUT, EXPIRED, COMPLETED_WITH_ERRORS, or INTERRUPTED run would silently return the dead run as isCached: true, instead of clearing the key and creating a fresh run. The root cause was twofold: - findRunsByIdempotencyKeys SQL query did not SELECT the status column - IdempotencyKeyRunMatch type did not include status Fix: - Add status to IdempotencyKeyRunMatch and the backing SQL query - Add shouldIdempotencyKeyBeCleared guard in #prepareRunData, mirroring the single-trigger path Tests: - 21 unit tests covering all TaskRunStatus values - Integration tests verifying status is returned from the query - Edge case coverage for COMPLETED_SUCCESSFULLY, CANCELED, and RETRYING_AFTER_FAILURE (all correctly remain cached)
|
|
Hi @Jaimin2687, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughThe run store now returns task run status with idempotency-key matches. The batch trigger uses ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
| // Mirror the single-trigger path (IdempotencyKeyConcern.handleExistingRun): | ||
| // if the cached run is in a terminal failure state (CRASHED, SYSTEM_FAILURE, | ||
| // TIMED_OUT, EXPIRED, COMPLETED_WITH_ERRORS, INTERRUPTED), clear the | ||
| // idempotency key and re-trigger instead of returning the dead run. | ||
| if (shouldIdempotencyKeyBeCleared(cachedRun.status as TaskRunStatus)) { | ||
| expiredRunIds.add(cachedRun.friendlyId); | ||
|
|
||
| return { | ||
| id: await this.mintChildFriendlyId(environment, childAnchor, item.options?.region), | ||
| isCached: false, | ||
| idempotencyKey: item.options?.idempotencyKey ?? undefined, | ||
| taskIdentifier: item.task, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟡 Missing server release note for user-facing fix
This user-facing behavior change to batch triggers ships only server code under apps/webapp and internal-packages, with no .server-changes/ note added. The repository guidelines require a .server-changes/ entry for user-facing server-only changes, so this fix will be absent from release notes.
Prompt for agents
CONTRIBUTING.md and AGENTS.md require a .server-changes/ file for user-facing server-only changes (changes under apps/webapp with no package changes). This PR changes observable batchTrigger behavior (failed runs are now re-triggered instead of returned as a dead cached run) and touches only apps/webapp and internal-packages. Add a markdown file under .server-changes/ (e.g. fix-batch-trigger-stale-failed-runs.md) with frontmatter `area: webapp` and `type: fix`, and a one-line user-facing description. See .server-changes/README.md for the exact format.
Was this helpful? React with 👍 or 👎 to provide feedback.
What happened
I noticed that
batchTriggerand singletriggerbehave differently when an idempotency key points to a run that already failed.With single
trigger, if a previous run with the same idempotency key ended up in a terminal failure state (CRASHED,SYSTEM_FAILURE,TIMED_OUT,EXPIRED,COMPLETED_WITH_ERRORS,INTERRUPTED), the SDK correctly clears the key and creates a fresh run. This works becauseIdempotencyKeyConcern.handleExistingRuncallsshouldIdempotencyKeyBeCleared(status)before deciding whether to return a cached result.With
batchTrigger, that check was missing entirely. The batch path inBatchTriggerV3Service.#prepareRunDataonly checked time-based expiration (idempotencyKeyExpiresAt < now), so a failed run would get returned asisCached: true— silently handing back a dead run that will never produce output.The root cause turned out to be pretty simple: the SQL query backing
findRunsByIdempotencyKeysnever selected thestatuscolumn, and theIdempotencyKeyRunMatchtype didn't include it. So even if someone wanted to add the check, the data wasn't there.What this PR does
Three small changes:
Added
statusto the query and its return type —IdempotencyKeyRunMatchinrun-store/src/types.tsnow includesstatus: string, and the raw SQL inPostgresRunStore.findRunsByIdempotencyKeysselects it.Added the missing status check in the batch path — After the existing expiry check in
#prepareRunData, there's now ashouldIdempotencyKeyBeCleared(cachedRun.status)guard that mirrors what the single-trigger path already does. If the cached run is in a failure state, we add it toexpiredRunIds(so the key gets cleared) and mint a new run ID.Added the import —
shouldIdempotencyKeyBeClearedwas already exported fromtaskStatus.ts, just not imported in the batch service.✅ Checklist
Testing
Unit tests — Added 21 tests in
batchTriggerIdempotencyStatusCheck.test.tsthat cover everyTaskRunStatusvalue againstshouldIdempotencyKeyBeCleared:true(key should be cleared, run re-triggered)false(run stays cached)COMPLETED_SUCCESSFULLY(valid cache),CANCELED(user-intentional),RETRYING_AFTER_FAILURE(still in progress), andEXPIREDIntegration tests — Extended the existing
PostgresRunStore.findRunsByIdempotencyKeys.test.tswith testcontainers:statusassertions to the existing test (catches future regressions if someone removes the column)COMPLETED_SUCCESSFULLYstatus is returned correctlyBuild verification:
pnpm run typecheck --filter webapp✅pnpm run build --filter @internal/run-store✅pnpm run format✅pnpm run lint:fix✅ (0 warnings, 0 errors)How to reproduce the bug manually:
Changelog
Batch triggers with idempotency keys now correctly re-trigger when a previous run failed, matching the existing single-trigger behavior. Previously, calling
batchTriggerwith an idempotency key that pointed to a crashed or failed run would silently return the dead run instead of starting a new one.Screenshots
N/A — backend-only change, no UI impact.
💯