feat: support retry-after header #142
Conversation
src/index.ts
Outdated
| // Or HTTP date time string | ||
| const dateTime = Date.parse(header); | ||
| if (!Number.isNaN(dateTime)) { | ||
| return dateTime - Date.now() + 3000; // Add 3sec buffer |
There was a problem hiding this comment.
Huh - why the 3 second buffer?
There was a problem hiding this comment.
Removed that now as yeah it's not necessary :)
| ]; | ||
| interceptorId = rax.attach(); | ||
| const res = await axios({url}); | ||
| assert.strictEqual(res.data, 'toast'); |
There was a problem hiding this comment.
This isn't actually testing if the delays work. I know it's annoying, but could I trouble you to set up a test with a sinon timer, and verify it's doing what we expect? I've had to write similar tests in the past if you're looking for an example:
https://github.com/JustinBeckwith/linkinator/blob/main/test/test.retry.ts#L83
There was a problem hiding this comment.
Done using the same pattern - also set a higher default retry delay on the rax config to make sure the Retry-After value is actually being used
test/index.ts
Outdated
| try { | ||
| const cfg: rax.RaxConfig = {url, raxConfig: {maxRetryAfter: 1000}}; | ||
| await axios(cfg); | ||
| } catch (e) { |
There was a problem hiding this comment.
I know I may be using this pattern elsewhere in the test, but it's less than ideal. Would you mind using an assert.rejects instead?
| @@ -78,7 +82,8 @@ describe('retry-axios', () => { | |||
| assert.fail('Expected to throw'); | |||
| }); | |||
|
|
|||
| it('should retry at least the configured number of times', async () => { | |||
| it('should retry at least the configured number of times', async function () { | |||
| this.timeout(10000); | |||
There was a problem hiding this comment.
Had to change the timeouts to the old function style after getting a weird error on the ci tests - https://github.com/JustinBeckwith/retry-axios/pull/142/checks?sha=f6f6c8d1bbe8b2a6da5301ee457fd6fe0970e121
|
Anything we can do to get this feature merged? :) |
|
@JustinBeckwith @blakeyp Any updates on this PR? |
|
@JustinBeckwith @blakeyp Thanks a lot :) |
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adds functionality to check for Retry-After header in error response and parse the value if it's available (as per https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After)
maxRetryAfter(default 5 mins)checkRetryAfterconfig param