-
-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use X-RateLimit-Reset
header to determine retry delay
#608
Comments
If I understand this correctly, it's basically a small optimization where the server can inform the client ahead of time that the next request would receive a That's kind of interesting, although I'm not really sure what we would do with this information. It implies that we would have to keep track of state across multiple The whole proposal seems poorly designed to me.
Given the above, plus the fact that it's not widely used beyond a few big names, my position is that until this is standardized, it should be implemented in a service client. See |
ky already has this logic as part of the This feature request is only about reading a couple more headers, I think. |
It's true that we handle However, We could certainly treat As far as I can tell, the whole point of the proposal is really to make clients react to I guess we could implement partial support and just handle error responses. Don't all of these APIs return |
I'll clarify my request by updating the title, that's not what I was suggesting.
Not in GitHub's case: https://github.com/orgs/community/discussions/24760 |
X-RateLimit-Reset
header to determine retry delay
Interesting! To me, that seems like pure laziness on their part. I mean, if At any rate 🙃, I can get behind treating these headers equivalently. Even though I don't think that's what the IETF proposal implies. We should probably do |
I'm currently using the Twitch API which implements I think a more general approach would be to allow optional custom parsing with access to the headers that returns the milliseconds to retry after. something like this: await ky.get('https://foo.com', {
retry: {
parseRetryHeader: (headers) => {
const ratelimit_reset_epoch = headers.get('Ratelimit-Reset')
return new Date(Number(ratelimit_reset_epoch) * 1000) - new Date()
}
}
}) What do you think? 🤔 |
We already support timestamps for |
The standards say that neither we, nor GitHub et al., should be using the See: RFC 6648 From MDN:
I opened a PR that does support it, though. To be pragmatic. |
Closes sindresorhus#608 RateLimit-Reset and X-RateLimit-Reset are used as a fallback for the Retry-After header to help support non-standard APIs.
Closes sindresorhus#608 `RateLimit-Reset` and `X-RateLimit-Reset` are used as a fallback for the `Retry-After` header to help support non-standard APIs.
Used by GitHub and X, apparently:
https://medium.com/@guillaume.viguierjust/rate-limiting-your-restful-api-3148f8e77248
Also proposed (and expired) to IETF:
https://datatracker.ietf.org/doc/draft-ietf-httpapi-ratelimit-headers/
It seems that the rate-limiting retry logic is already in ky, but no mention of these headers was found in the repo.
The text was updated successfully, but these errors were encountered: