Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for the RateLimit-Reset header #618

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Add support for the RateLimit-Reset header #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37b7a9b
Add support for RateLimit-Reset header
sholladay Aug 11, 2024
8246a07
Handle timestamps and more header names
sholladay Aug 12, 2024
ff41203
Tweak wording for consistency
sholladay Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ An object representing `limit`, `methods`, `statusCodes`, `afterStatusCodes`, an

If `retry` is a number, it will be used as `limit` and other defaults will remain in place.

If the response provides an HTTP status contained in `afterStatusCodes`, Ky will wait until the date or timeout given in the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header has passed to retry the request. If the provided status code is not in the list, the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header will be ignored.
If the response provides an HTTP status contained in `afterStatusCodes`, Ky will wait until the date, timeout, or timestamp given in the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header has passed to retry the request. If `Retry-After` is missing, the non-standard [`RateLimit-Reset`](https://www.ietf.org/archive/id/draft-polli-ratelimit-headers-05.html#section-3.3) header is used in its place as a fallback. If the provided status code is not in the list, the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header will be ignored.

If `maxRetryAfter` is set to `undefined`, it will use `options.timeout`. If [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header is greater than `maxRetryAfter`, it will use `maxRetryAfter`.

Expand Down
8 changes: 7 additions & 1 deletion source/core/Ky.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,17 @@ export class Ky {
throw error;
}

const retryAfter = error.response.headers.get('Retry-After');
const retryAfter = error.response.headers.get('Retry-After')
?? error.response.headers.get('RateLimit-Reset')
?? error.response.headers.get('X-RateLimit-Reset') // GitHub
?? error.response.headers.get('X-Rate-Limit-Reset'); // Twitter
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')) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this hardcoded to "2024-01-01"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1700000000 milliseconds 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

Copy link
Collaborator Author

@sholladay sholladay Aug 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// A large number is treated as a timestamp (fixed threshold protects against clock skew)
after -= Date.now();
}

const max = this._options.retry.maxRetryAfter ?? after;
Expand Down
2 changes: 1 addition & 1 deletion source/types/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export type KyOptions = {

If `retry` is a number, it will be used as `limit` and other defaults will remain in place.

If the response provides an HTTP status contained in `afterStatusCodes`, Ky will wait until the date or timeout given in the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header has passed to retry the request. If the provided status code is not in the list, the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header will be ignored.
If the response provides an HTTP status contained in `afterStatusCodes`, Ky will wait until the date or timeout given in the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header has passed to retry the request. If `Retry-After` is missing, the non-standard [`RateLimit-Reset`](https://www.ietf.org/archive/id/draft-polli-ratelimit-headers-02.html#section-3.3) header is used in its place as a fallback. If the provided status code is not in the list, the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header will be ignored.

If `maxRetryAfter` is set to `undefined`, it will use `options.timeout`. If [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After) header is greater than `maxRetryAfter`, it will cancel the request.

Expand Down
65 changes: 65 additions & 0 deletions test/retry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,71 @@ test('respect Retry-After: 0 and retry immediately', async t => {
await server.close();
});

test('RateLimit-Reset is treated the same as Retry-After', async t => {
let requestCount = 0;

const server = await createHttpTestServer();
server.get('/', (_request, response) => {
requestCount++;

if (requestCount === defaultRetryCount + 1) {
response.end(fixture);
} else {
const header = (requestCount < 2) ? 'RateLimit-Reset' : 'Retry-After';
response.writeHead(429, {
[header]: 1,
});

response.end('');
}
});

await withPerformance({
t,
expectedDuration: 1000 + 1000,
async test() {
t.is(await ky(server.url).text(), fixture);
},
});

t.is(requestCount, 3);

await server.close();
});

test('RateLimit-Reset with time since epoch', async t => {
let requestCount = 0;

const server = await createHttpTestServer();
server.get('/', (_request, response) => {
requestCount++;

if (requestCount === defaultRetryCount + 1) {
response.end(fixture);
} else {
const twoSecondsByDelta = 2;
const oneSecondByEpoch = (Date.now() / 1000) + 1;
response.writeHead(429, {
'RateLimit-Reset': (requestCount < 2) ? twoSecondsByDelta : oneSecondByEpoch,
});

response.end('');
}
});

await withPerformance({
t,
expectedDuration: 2000 + 1000,
async test() {
t.is(await ky(server.url).text(), fixture);
},
});

t.is(requestCount, 3);

await server.close();
});

test('respect 413 Retry-After', async t => {
const startTime = Date.now();
let requestCount = 0;
Expand Down