-
Notifications
You must be signed in to change notification settings - Fork 18
Retry on 429 and respect the Retry-After header #31
Conversation
9818691
to
94a5ad4
Compare
5e3d2e8
to
f7b66ae
Compare
94a5ad4
to
edaed29
Compare
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
There was a problem hiding this 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
edaed29
to
b38cac9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
779ee71
to
3c148f9
Compare
Based on #30
It doesn't support
Retry-After: <http-date>
for now