search

beam-community / stripity-stripe

An Elixir Library for Stripe
Other
985 stars 353 forks source link

Retries with delete subscription #711

Closed jfcalvo closed 2 years ago

jfcalvo commented 2 years ago

I think there is a problem with retries and requests that perform destructive actions like delete a subscription.

In my case I'm trying to delete a subscription and I'm getting an 404 error with resource_missing. These are the logs for the subscription at Stripe:

Captura de pantalla 2022-01-17 a las 13 13 28

I think the library is doing the first request that is taking more time than expected, then retrying again (twice) but the first request already deleted the subscription so the two last request are returning an error and finally returning the error of the last request.

The retrying policy should not be a problem with idempotent request but for requests like delete it's a source of possible problems.

What should be the correct solution for this? Disable retries? Increase base_backoff or max_backoff?

I'm using stripy_stripe 2.12.1.

snewcomer commented 2 years ago

👋 Thx for filing the issue! In order for us to retry, we 1. need a response and 2. need it needs to be of the following -

https://github.com/beam-community/stripity_stripe/blob/829b3714e0447efe76b5065e9a4f1600a68aba22/lib/stripe/api.ex#L429-L436

Do you happen to know which one of these is being hit?

snewcomer commented 2 years ago

Also, I would have expected our use of idempotency_keys would prevent this type of issue. Do you have a log of the headers from those requests?

https://stripe.com/docs/api/idempotent_requests

snewcomer commented 2 years ago

Another question would be is if you have recv_timeout configured? That might be a source of an issue here?

config :stripity_stripe, hackney_opts: [{:connect_timeout, 1000}, {:recv_timeout, 5000}]
snewcomer commented 2 years ago

There are two notable discussion points.

  1. idempotency keys should only be used for POST requests. We may be doing it for not just POST requests 😓. I'll check and fix
  2. Idempotency keys on DELETE should have no effect (same with PUT). Ideally, we need the logs to see the Idempotency-Key and if we generated a different one. Any retry should use the same key - our logic only adds the idempotency key once and retries with the same