Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Retry on 429 and respect the Retry-After header #31

Merged
merged 5 commits into from
Mar 3, 2020

Conversation

nkzawa
Copy link
Contributor

@nkzawa nkzawa commented Feb 26, 2020

Based on #30

It doesn't support Retry-After: <http-date> for now

Copy link
Member

@lucleray lucleray left a comment

Choose a reason for hiding this comment

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

Looks good!

if ((res.status >= 500 && res.status < 600) || res.status === 429) {
// NOTE: doesn't support http-date format
const retryAfter = parseInt(res.headers.get('retry-after'), 10);
if (retryAfter) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a maximum value for retryAfter? (ie. not retry if retryAfter is 2 hours). Maybe ~20s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the maxRetryAfter option which defaults to 20.

Copy link
Member

@javivelasco javivelasco left a comment

Choose a reason for hiding this comment

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

@lucleray sounds good to me

@nkzawa nkzawa force-pushed the 429-and-retry-after branch from edaed29 to b38cac9 Compare March 2, 2020 05:04
@nkzawa nkzawa requested a review from lucleray March 2, 2020 05:33
Copy link
Member

@lucleray lucleray left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@nkzawa nkzawa force-pushed the 429-and-retry-after branch from 779ee71 to 3c148f9 Compare March 3, 2020 11:08
@nkzawa nkzawa changed the base branch from fix-retry-count-and-error to master March 3, 2020 11:08
@nkzawa nkzawa merged commit 559a81b into master Mar 3, 2020
@styfle styfle deleted the 429-and-retry-after branch March 3, 2020 14:15
@sandinmyjoints sandinmyjoints mentioned this pull request Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants