Lock webhook handlers against concurrent duplicate deliveries - #321
Merged
Conversation
Replace `! lessThan` with `greaterThanOrEqual` for readability in the aftercare chargeback branch, and add a stale-read regression test that proves concurrent deliveries dispatch `ChargebackReceived` only once. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Protects customers and merchants from the financial side effects of duplicate webhook deliveries. Without this change, a retried or concurrent webhook can cause a customer to be credited twice for the same failed payment, receive two refund orders for a single refund, or have a chargeback counted more than once. This PR makes the paid, failed, refund, and chargeback webhook flows safe to deliver more than once, with no schema changes, migrations, cache locks, or new drivers required.
Changes
Order::handlePaymentPaid/handlePaymentFailed: reload the order withlockForUpdate, bail if alreadypaid/failed, and only dispatchOrderPaymentPaid/OrderPaymentFailedwhen this call actually transitioned the row. Payment lookup switches tofindByPaymentIdOrFailnow that first-payment idempotency guarantees the row exists (Fix idempotent first payment webhooks #320).Refund::handleProcessed/handleFailed: lock the refund, bail unless stillpending, and gate theRefundProcessed/RefundFailedevent on the winning transaction so the compensating order and item hooks run once.AftercareWebhookControllerchargeback branch: wrap the charged-back comparison and update in a transaction that locks the payment row before recomputing the delta, so concurrent deliveries cannot both dispatchChargebackReceivedfor the same amount.Tests
OrderTestandRefundTest. Each handler is invoked on an outdated model instance whose row has already transitioned, and the test asserts no duplicate event, no duplicate credit/refund order, and no second increment.ChargebackStaleReadPaymentfixture and an aftercare test proving a staleamount_charged_backread does not produce a duplicateChargebackReceived.AftercareWebhookControllerTest.