TrueLayer / reqwest-middleware

Wrapper around reqwest to allow for client middleware chains.
Apache License 2.0
257 stars 78 forks source link

reqwest-retry: does it support the Retry-After header? #146

Open bbigras opened 5 months ago

bbigras commented 5 months ago

Does it support Retry-After?

The Retry-After response HTTP header indicates how long the user agent should wait before making a follow-up request. There are three main cases this header is used:

When sent with a [503](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/503) (Service Unavailable) response, this indicates how long the service is expected to be unavailable.
When sent with a [429](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429) (Too Many Requests) response, this indicates how long to wait before making a new request.
When sent with a redirect response, such as [301](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/301) (Moved Permanently), this indicates the minimum time that the user agent is asked to wait before issuing the redirected request.
cbeck88 commented 4 months ago

The crate is very customizable, but this particular thing I don't think is possible right now.

The idea right now is that the RetryStrategy looks at what happened and decides whether to retry, and the RetryPolicy decides how to long wait for, and it doesn't get to see the response itself when it makes that judgment, so I don't think you could make a custom RetryPolicy that does this right now.

The place where you would want to read the header might be like here: https://github.com/TrueLayer/reqwest-middleware/blob/2927624652097b687f2970fba2cf454d04b1eb1a/reqwest-retry/src/retryable_strategy.rs#L113

and you can see that the header is not inspected. But even if it is, with this trait you can only return whether to retry or not, and there's nowhere to stick the server-suggested deadline.

If you wanted to add support, you could try adding a new enum variant to Retryable that is like, transient with server-suggested deadline, and then making the middleware object use that info. That would be a breaking change though. It’s probably simpler to make a new middleware tbh

joebowbeer commented 4 months ago

See https://github.com/melotic/reqwest-retry-after