Handle P2002 race in createDateTimeWaitpoint#4134
Draft
claude[bot] wants to merge 1 commit into
Draft
Conversation
Under concurrency, two requests with the same user-provided idempotencyKey can both pass the findFirst check and attempt the INSERT, violating the @@unique([environmentId, idempotencyKey]) constraint and surfacing a P2002 as a 500. Catch P2002 (only when a user-provided key is present), fetch the row created by the concurrent winner, and return it as cached with idempotent semantics instead of blindly retrying.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog
Before: Two genuinely-concurrent requests carrying the same user-provided
idempotencyKeyfor a DATETIME waitpoint could both pass thefindFirstexistence check, both attempt the INSERT, and the loser would violate@@unique([environmentId, idempotencyKey]). Prisma raised aP2002(PrismaClientKnownRequestError), which surfaced as a 500.After: The race is handled idempotently. The request that loses the INSERT race now catches the
P2002, fetches the row created by the concurrent winner, and returns it as{ waitpoint, isCached: true }— the same shape returned when an existing waitpoint is found up front. No error is surfaced.How
createDateTimeWaitpointdid a non-atomic check-then-upsert. Theupsertis now wrapped in atry/catch. OnP2002— and only when a user-providedidempotencyKeyis present (the auto-generatednanoidcase effectively never collides) — it re-reads the row by{ environmentId, idempotencyKey }and returns it as cached. Because the concurrent winner already created the row and enqueued itsfinishWaitpointworker job, the recovery path returns the existing row without re-enqueuing, avoiding a double-scheduled job. A plain retry loop (as used for the auto-key path) is deliberately avoided here since a user-provided key would just re-collide.The sibling
createManualWaitpointalready guards the identicalupsertagainstP2002(via a retry loop), so this brings the DATETIME path in line. The two functions are intentionally left un-unified to keep this change tight; unifying them is a possible follow-up.Root cause: non-atomic
findFirst-then-upsertincreateDateTimeWaitpoint—internal-packages/run-engine/src/engine/systems/waitpointSystem.ts:229.Testing
pnpm --filter @internal/run-engine typecheck(i.e.tsc --noEmit -p tsconfig.build.json) — passes cleanly after generating the Prisma client and building workspace dependencies.internal-packages/run-engine/src/engine/tests/waitpoints.test.ts,waitpointRace.test.ts) run viacontainerTestfrom@internal/testcontainers, which requires Docker-backed Postgres/Redis containers. A meaningful P2002 regression test needs a live DB to actually trigger the unique-constraint violation under real concurrency; that setup is too heavy to add reliably here without introducing flakiness, so it is deferred.