Add support for the RateLimit-Reset header
#618
Conversation
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.
sholladay
changed the title
RateLimit-Reset header
sholladay
changed the title
RateLimit-Reset headerRateLimit-Reset header
|
@fregante does this satisfy your requirements? |
| if (retryAfter && this._options.retry.afterStatusCodes.includes(error.response.status)) { | ||
| let after = Number(retryAfter) * 1000; | ||
| if (Number.isNaN(after)) { | ||
| after = Date.parse(retryAfter) - Date.now(); | ||
| } else if (after >= Date.parse('2024-01-01')) { |
There was a problem hiding this comment.
Why is this hardcoded to "2024-01-01"?
There was a problem hiding this comment.
It is stupid, yes.
See the comment on the next line (the linter would not let me put it above).
Some RateLimit-Reset implementations in the wild use delta seconds, while others use timestamps. Both are integers, with no easy way to disambiguate them other than, essentially, how large the number is. Generally speaking, we would expect delta seconds to be a small number (because it's referring to seconds from now) and a timestamp to be a large number (because it's referring to seconds from epoch). Ky wants to normalize these values to delta seconds. So we need to detect timestamps and convert them.
The most obvious approach might be to check if after is close to Date.now(), but that is susceptible to clock skew and incorrectly set clocks. Such bugs also have a history of leading to security vulnerabilities. So, we use a constant threshold instead.
How large should after be for us to consider it a timestamp? That's... very arbitrary. @fregante suggested we use 1700000000 in this comment: #618 (comment)
1700000000 milliseconds is a little less than 20 days, which should be fine in the vast majority of cases. But if someone wants to set a 6 month retry delay and hope the computer stays up for that long... who am I to stop them? I figured that Date.parse('2024-01-01'), being the beginning of the year in which this feature was implemented, is hopefully more readable, allows for extremely long retry delays, and makes it obvious that the value is arbitrary and not based on some standard (I don't think there is a standard for this).
I'm very open to improvements here, suggestions welcome! Would be nice to put it in a const with a self-documenting name, but I couldn't think of a good name.
There was a problem hiding this comment.
1700000000milliseconds is a little less than 20 days
Retry-After uses seconds, not milliseconds. Since this value is meant to differentiate that header from an epoch-based value, this is fine. No server will ask you to retry in 53 years (unless you're somehow querying Deep Thought)
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
There was a problem hiding this comment.
Retry-After and RateLimit-Reset both use seconds (except for Mulesoft, apparently 😡). However, we convert them both to milliseconds.
Indeed, 1700000000 seconds is plenty high enough. I could've just added a few more zeros for milliseconds. I just thought Date.parse('2024-01-01') (which is 1704067200 seconds) had a certain logic to it. Perfectly happy to change it, though.
Closes #608
This PR adds automatic handling for non-standard rate limiting headers that are used by some APIs (e.g. GitHub and Twitter) instead of
Retry-After.The following headers are now supported as a fallback for
Retry-After:RateLimit-Resetas proposed by https://www.ietf.org/archive/id/draft-polli-ratelimit-headers-05.html#section-3.3X-RateLimit-Resetas implemented by GitHubX-Rate-Limit-Resetas implemented by TwitterDates, timeouts (delta seconds), and timestamps (seconds since epoch) are all supported.
Timestamps in milliseconds are not currently supported, as none of the popular APIs use it.
More variants are discussed here: ietf-wg-httpapi/ratelimit-headers#25