fix: 429 runaway retry. retry_after is in seconds and so is sleep

[sadleb]

Summary

The retry_after response returned with a 429 is in seconds according to the documentation and so is the sleep time. We shouldn't be converting it to milliseconds. This PR mimics what the handle_preemptive_rl method does for X-RateLimit-Reset-After which is also in seconds.

I don't know how to reproduce this yet, but it happened twice today and generated hundreds of thousands of retry requests milliseconds apart because the sleep was essentially 0. We had to reboot the server to stop it. Here is what that looked like in our Honeycomb.io monitoring dashboard:

We've added some more instrumentation to see the RestClient response headers so if it happens again, maybe we can get a better idea of what is going on, but regardless this seems like a very-low-risk change that would prevent other's from getting into this out of control retry loop.

Fixed

• Retry wait_seconds when handling RestClient::TooManyRequests

[Daniel-Worrall]

Yeah, seems this came in discord/discord-api-docs#2097 and it was missed this whole time. I'm quite worried we hadn't had reports of this before now though

[sadleb]

Yeah, seems this came in discord/discord-api-docs#2097 and it was missed this whole time. I'm quite worried we hadn't had reports of this before now though

I'll let you know if we find out more about why we're getting a 429 in the first place.

[sadleb]

Yeah, seems this came in discord/discord-api-docs#2097 and it was missed this whole time. I'm quite worried we hadn't had reports of this before now though

I'll let you know if we find out more about why we're getting a 429 in the first place.

Can I expect this to be merged anytime soon? Trying to decide if I need to monkey patch or something in the meantime. Thanks for the quick response by the way!

[swarley]

I should be able to merge this tonight, I just need to check that the information returned in the headers isn't more precise (because there should also be a Retry-After header and I remember there being a discrepancy between the two values)

[sadleb]

I should be able to merge this tonight, I just need to check that the information returned in the headers isn't more precise (because there should also be a Retry-After header and I remember there being a discrepancy between the two values)

Gotcha. Thanks so much!

[swarley]

It appears that the header value is the least precise. Thanks for the PR! 🚀