integrallis / stripe_event

Stripe webhook integration for Rails applications.
https://rubygems.org/gems/stripe_event
MIT License
840 stars 107 forks source link

Concurrency and DB race conditions best practices #104

Closed chrismanderson closed 6 years ago

chrismanderson commented 6 years ago

Hello, love this library it has been enormously useful to us. This might be more of a Rails question than anything, but I figured someone here has run into the same issue.

We deal with Stripe subscriptions, and have a stripe_event listener for all invoice actions where I create/update our internal Invoice model with data from the Stripe invoice.

events.subscribe 'invoice.', Subscription::InvoiceProcessor.new

In that processor, I store/update the invoice passed via the webhoook.

  def self.store(stripe_invoice_object)
    subscription = Subscription.find_by(external_id: stripe_invoice_object.subscription)

    invoice = Subscription::Invoice.find_or_initialize_by(
      number: stripe_invoice_object.number
    )
    invoice.from_stripe(stripe_invoice_object)
    invoice.save
  end

I basically always want the invoice to be 1) created if it does not exist and 2) updated if it does. 99% of the time this works well; but on occasion, I get Postgres duplicate key errors (I have a DB uniqueness validation on the invoice number) as the webhooks from Stripe sometimes arrive at the same time.

Any suggestions on preventing these errors? I could rescue the PG::UniqueViolation error and run my store method again; which is probably the simplest but also feels wrong for some reason. I could switch to find_or_create as well, but I'd kind of like to avoid the two DB calls to save, though again, might not really matter if it avoids this uniqueness issue in the first place.

Any suggestions would be greatly appreciated. Thanks!

rmm5t commented 6 years ago

One key aspect to working with Stripe webhooks is that you must make sure that everything you do is idempotent.

Your instinct to use find_or_create is correct (although, find_or_create_by has an implicit race condition itself). In theory, you should be able to wrap find_or_create_by in a transaction to solve that. Worrying about two DB calls is kind of 🤪 -- especially considering that the webhook events aren't user-facing, but from a DB perspective there are better ways to handle this.

Because you're using PostgreSQL, you can instead issue an UPSERT. active_record_upsert might be of value there.

More information about UPSERTs on PostgreSQL can be found here: https://wiki.postgresql.org/wiki/UPSERT

chrismanderson commented 6 years ago

Thanks, that's some great feedback. I've seen active_record_upsert before and this might be a great time to try it out (would love to make it into Rails proper). Race conditions, always the best. And great point about worrying about two DB calls; it really doesn't matter at all for a job like this. Thanks for the tips!