Retry mechanism probably does nothing

[mgsloan]

Current Logic

The current retry logic is:

• Don't retry if the Stripe-Should-Retry header is false or absent.

• Don't retry if the status code is a client error (within 4xx).

I believe that this is probably causing no retries to ever be done, unless Stripe-Should-Retry is sometimes set to true on 5xx errors.

Correct logic?

My read of the docs is that instead the logic should be:

• Always retry if the Stripe-Should-Retry header is true, and never retry if it is false.

• When the header is absent, only retry for 429 status responses.

Relevant docs

From the Stripe-Should-Retry docs:

Stripe-Should-Retry not set in the response indicates that the API can’t determine whether or not it can retry the request. Clients should fall back to other properties of the response (like the status code) to make a decision.

I wish it would just say right there how this fall back should behave!

There is at least one status code that clearly should be retried. From the status codes table:

429 Too Many Requests

Too many requests hit the API too quickly. We recommend an exponential backoff of your requests.

The current logic retries 5xx statuses, which the section on server statuses specifically says should not be done:

As with user errors, the idempotency layer caches the result of POST mutations that result in server errors (specifically 500s, which are internal server errors), so retrying them with the same idempotency key usually produces the same result. The client can retry the request with a new idempotency key, but we advise against it because the original key may have produced side effects.

Treat requests that return 500 errors as indeterminate. Although Stripe tries to reconcile their partial state in the most appropriate manner and also fire webhooks for new objects that are created, ideal results are not guaranteed.

It's probably not an actual problem for the current logic, since retries are only happening if Stripe-Should-Retry is true.

[arlyon]

Thanks for this. If you have the opportunity to write a test or provide some logs that would help speed up the implementation process.

[mgsloan]

@arlyon Welcome! Here's the retry logic and tests we're using in a custom stripe client - https://gist.github.com/mgsloan/fc24c415c2c5aae79741332fca69b442 - should be straightforward to adapt it to async-stripe, though this only supports the one retry strategy being used.