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

The Stripe Webhook Callback should fail more gracefully #2626

Open
zspencer opened this issue Oct 4, 2024 · 4 comments
Open

The Stripe Webhook Callback should fail more gracefully #2626

zspencer opened this issue Oct 4, 2024 · 4 comments
Labels
🐞 bug Something isn't working good first issue

Comments

@zspencer
Copy link
Member

zspencer commented Oct 4, 2024

Our current implementation can leave orders in strange spots if they fail in the middle.

On the one hand, this is good because we don't want to redo important steps that have already happened.

On the other, it's not great because if step B updates the order to paid and step C fails then D and E don't happen and never will.

It would probably be better to either:

  1. Put this all in a transaction, and if any part fails don't commit the transaction (not sure how to do that, since sidekiq uses redis...)
  2. Figure out a better way to ensure each piece of the order completion process can be eventually resolved if any one point fails

Thoughts?

@zspencer zspencer added 🐞 bug Something isn't working good first issue labels Oct 4, 2024
@zspencer zspencer changed the title The Stripe Webhook Callback should be in a Transaction The Stripe Webhook Callback should fail more gracefully Oct 4, 2024
@joetho786
Copy link

@zspencer I would like to work on this

@zspencer
Copy link
Member Author

zspencer commented Oct 4, 2024

We'd love the help!

Do you have an idea for how you'd like to go about it?

@joetho786
Copy link

joetho786 commented Oct 5, 2024

@zspencer, I was thinking to wrap the block in a transaction like you suggested and use after_commit callback to run the sidekiq jobs. This would ensure that only after the transaction succeeds the next steps would be executed.

To be specific the below lines would be placed inside the after_commit callback

Order::ReceivedMailer.notification(order).deliver_later
order.events.create(description: "Notifications to Vendor and Distributor Sent")
        Order::PlacedMailer.notification(order).deliver_later
order.events.create(description: "Notification to Buyer Sent")
SplitJob.perform_later(order: order)

We can use after_rollback callback to handle use cases when transaction gets rolled back
I have referred the docs for this: https://api.rubyonrails.org/classes/ActiveRecord/Transaction.html

Do let me know if this approach would suffice or if I am missing something

@Kroch4ka
Copy link
Contributor

Kroch4ka commented Oct 6, 2024

Hello!

Can you please take a look at such a solution)

PR: #2634

Thanks in advance!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working good first issue
Projects
None yet
Development

No branches or pull requests

3 participants