Checkout webhooks processed twice since Tuesday's deploy
Verdict
Move shipping out of the webhook request and make the warehouse call idempotent. A better lock won't fix this. In one database transaction, record the provider event ID (unique) and a durable fulfillment job, then return 200. A worker then ships with a stable idempotency key and saves the shipment ID before sending the receipt.
First step this week: Ask your warehouse vendor whether the ship endpoint accepts an idempotency key, and what its scope and retention are.
Confidence
Medium. Claude, GPT and Grok agree on how the bug happens and on the fix, but Gemini thinks the background worker is overkill, and Grok, arguing the other side, pointed out gaps in the evidence (the 10 s retry is only a code comment, and a 2% duplicate rate is hard to square with an 11.8 s p95).
Agreement
2 of 4 agree; 2 partly agree (Gemini, Grok).
What changed under review
- Claude first said to ship the atomic claim and event-ID table today and background shipping afterwards; after Claude's own review flagged that a claim alone can strand orders, GPT withdrew the staged rollout and required the event record and durable job to be committed together before the 200.
- Claude's first fix used WHERE status='pending'; after Claude's review and Grok noted 'pending' appears nowhere in the code, GPT said neither 'pending' nor NOT IN ('paid','processing') is safe and the real status value must be read first.
- Claude's review proposed returning 5xx/409 when an order is already claimed so the provider keeps retrying; Grok rejected this as telling the provider paid orders failed, and GPT agreed provider retries are not a demonstrated recovery mechanism, so the plan returns 200 on a live claim.
Why
- The provider retries after 10 s, and on Tuesday warehouse p95 jumped from 2.1 s to 11.8 s. Retries now arrive while the first attempt is still shipping, and you can't answer in 10 s with a 12 s call on the request path. (webhook.js, line 7; logs.txt, line 9)
- The 'paid' check runs before the lock, and 'paid' is only written after shipping and the email. Attempt 2 reads the old state and ships again. (webhook.js, lines 14–19)
- The pg_advisory_xact_lock runs as its own statement with no BEGIN, so it is released as soon as that statement ends. The logs show the second ship starting at 14:02:21.020 while the first runs until 14:02:23.401, giving two shipment IDs (shp_99120, shp_99121). (webhook.js, line 16; logs.txt, lines 2–6)
Where they disagreed
- Is a background worker needed, or is a fix inside the handler enough? Gemini: ship the atomic update, event-ID dedupe and a warehouse idempotency key, keep shipping synchronous, and skip workers and sweepers. GPT, Grok and Claude: with an 11.8 s p95 against a 10 s retry window, nothing inside the request stops retries, so shipping has to be durable work outside the request. Not settled. To settle it: Confirm the provider's actual retry behaviour (whether it really retries at 10 s, and why) and whether your warehouse API accepts an idempotency key.
- What to return when an order is already claimed. Claude suggested returning 5xx/409 so provider retries act as recovery. Grok and GPT said return 200 and recover on your side. Settled: return 200 on a live claim. Recovery reships only after checking for an existing shipment, or flags the order for a person.
- Is the timing race proven for every duplicate? Claude, GPT and Grok's review say the logs prove it. Grok, arguing the other side, said the 10 s retry is a comment rather than a delivery log, that an 11.8 s p95 should give far more than 2% slow calls, and that two different event IDs for one order would get past event-ID dedupe. Not settled. To settle it: Diff Tuesday's deploy, confirm the provider's retry reason (timeout, 5xx or a duplicate subscription), and check whether retries always reuse the same event ID.
The case against
The causal story is stronger than the evidence. The 10 s retry is a code comment, not a delivery log. A proxy or function timeout could also cut the connection. A p95 of 11.8 s usually means far more than 2% of calls go over 10 s, so line 9 may be the wrong metric or another path may be involved. Diff Tuesday's deploy before blaming the warehouse. If the driver wraps statements, the lock isn't 'dead'. A smaller fix (a real row claim plus a stable warehouse key) is possible. The queue design is where the worse bug lives: dropped orders. Don't ship it until warehouse idempotency and crash-after-success behaviour are confirmed.
Not verified
- Whether your warehouse API supports idempotency keys, and with what scope and retention.
- How ./db behaves: whether it wraps statements in transactions, and whether db.query and db.orders.update share a connection.
- The real status value for an unpaid order ('pending' is a guess).
- That the provider really retries at 10 s (only a code comment), and how it handles 409/5xx or how long it keeps retrying.
- That every 1-in-50 duplicate is this same race, given the mismatch between the p95 and the duplicate rate.
What would change the answer
- Duplicates still happening when warehouse calls finish in under 10 s would point to a second cause, such as duplicate events or several app instances.
- The provider not actually retrying on a 10 s timeout, which would weaken the case for answering before shipping.
- ./db wrapping each request in one transaction, so the lock was actually being held.
- Tuesday's deploy adding a new endpoint, a second subscription or a handler change that explains the duplicates.
Next steps
- This week: ask your warehouse vendor whether the ship endpoint accepts an idempotency key, and what its scope and retention are.
- This week: look up one unpaid order and note its real status value.
- Confirm the provider's retry reason, check whether retries reuse the event ID, and diff Tuesday's deploy.
- Build it as one unit: event ID plus a durable fulfillment job in one transaction, then return 200. A worker does an atomic claim, counts attempts, calls the warehouse with a stable key, saves the shipment ID before the receipt email, and recovers stale claims only after checking for an existing shipment (or flags them for a person). Delete the pg_advisory_xact_lock line.
- Before rollout, test concurrent delivery of the same event, a crash between warehouse success and saving the shipment ID, and a crash before commit.
- Check orders currently in flight against warehouse records before the worker starts.
Each model's position (4)
Claude (Anthropic): Same cause: the warehouse slowed down, the provider retries at 10 s, the 'paid' check is stale, and the lock is released straight away. Its first plan was to claim the order in one atomic update and add an event-ID table today, with background shipping later. On review it warned that shipping the claim alone can lose orders silently, that 'pending' is a guessed status value, and that a background job must be durable, not in-memory.
GPT (OpenAI): Accepted Claude's silent-loss point and dropped the staged rollout. Ship everything as one unit: event record plus durable job in one transaction before the 200, a durable worker with atomic claim and recovery, idempotency at the warehouse, and separate tracking for the receipt. Using provider retries as recovery isn't proven. Neither 'pending' nor NOT IN is safe to copy.
Gemini (Google): Agrees on the cause. Its first fix was to wrap the lock, a re-check of the status, and the shipping in one explicit transaction. Later it called background workers, a 'processing' state and sweepers an overreaction. Ship the atomic update, event-ID dedupe and a warehouse idempotency key, keep shipping synchronous, and accept that the provider may log timeouts.
Grok (xAI): Agrees on the cause. Says the warehouse call itself must take a stable idempotency key and save the shipment ID before the email. You must return 200 before shipping. Rejected returning 5xx on a live claim. Arguing the other side, it questioned whether one log pair proves every case and asked for a check of Tuesday's deploy and the provider's real retry reason.