Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be resilient against duplicated webhook events #188

Closed
waiting-for-dev opened this issue Feb 23, 2023 · 3 comments · Fixed by #286
Closed

Be resilient against duplicated webhook events #188

waiting-for-dev opened this issue Feb 23, 2023 · 3 comments · Fixed by #286
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@waiting-for-dev
Copy link
Contributor

There's no guarantee that Stripe will send a webhook event only once (e.g., our server could timeout for the response while we've actually processed it; in that case, Stripe would retry). We need to ensure that we only process events once or that the subscribers are designed in an idempotent way.

Partially replaces #160

@loicginoux
Copy link
Contributor

Not saying it's the definite solution but as discussed in #189 (comment) having an internal state machine and validating transitions might solve this problem as well...

@waiting-for-dev
Copy link
Contributor Author

waiting-for-dev commented Apr 17, 2023

All of the webhook handlers we'll be shipping should be idempotent:

  • payment_intent.succeeded will do nothing if the payment is already completed.
  • payment_intent.payment_failed will do nothing if the payment is already canceled.
  • payment_intent.canceled will do nothing if the payment has already been voided.
  • charge.refunded will sync all refunds in an idempotent way once Avoid duplicated Solidus refunds #281 is merged.

At least for this initial release, it can be an acceptable compromise to ship with nothing more robust at a higher level. In order to provide enough flexibility for a user to build their own system, we could modify omnes to allow adapters to be configured at the subscriber class level and have a top-level subscriber class for Stripe webhook handling.

waiting-for-dev added a commit that referenced this issue Apr 17, 2023
By acquiring an update lock on the payment, we can ensure that only one
concurrent excecution of the webhook handler will manipulate it. That will avoid
duplicated log entries and obscure exceptions.

Closes #188
@waiting-for-dev
Copy link
Contributor Author

With #285, we also acknowledge race conditions for duplicated events. Once that's merged, we can close this issue and re-take the work on the topic if we find time to work on configuring subscriber-level adapters on Omnes (which shouldn't be a great effort).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants