retry requests when rate limit reached #315
Conversation
Adds support for retrying requests that fail with 429 using the `Retry-After` header, similar to what we do in godo. This is still WIP, since the README hasn't yet been update to describe the change in behaviour. I also still need to run some manual tests.
lib/droplet_kit/client.rb
Outdated
| # faraday-retry supports both the Retry-After and RateLimit-Reset | ||
| # headers, however, it favours the RateLimit-Reset one. To force it | ||
| # to use the Retry-After header, we override the header that it | ||
| # expects for the RateLimit-Reset header to something that we know | ||
| # we don't set. | ||
| rate_limit_reset_header: 'undefined' |
There was a problem hiding this comment.
See https://github.com/lostisland/faraday-retry/blob/41b7ea27e30d99ebfed958abfa11d12b01f6b6d1/spec/faraday/retry/middleware_spec.rb#L269-L274 for confirmation, this is surprising behaviour.
There was a problem hiding this comment.
Looks like we're providing a Unix timestamp and it's expecting seconds until?
There was a problem hiding this comment.
I've since spent some time testing this manually, and while you're right, I think this is still the best approach for the library.
I've restored the previous 429 status code handling if the Retry-After header is present, so that when the burst limit is reached, the client will wait, but if it's not, a rate limit error is raised. For this to work, this line is needed, since the faraday-retry middleware executes before the error handling logic. If it's not present, faraday-retry takes over and tries to wait until the RateLimit-Reset header, which can (in the absolute worst possible case) wait for an hour. The client being unresponsive for an hour makes it feel like the client is broken, so I opted to return an error in that case.
There was a problem hiding this comment.
Is there a way to override this altogether? Should this be conditional on retry_max being set to greater than 0?
There was a problem hiding this comment.
Update to be conditional on retry_max being set greater than 0.
|
The tests for Ruby 2.5 were failing in the setup stage when trying to install |
Adds support for retrying requests that fail with 429 using the
Retry-Afterheader, similar to what we do in godo.