diff --git a/docs/docs/api/RetryHandler.md b/docs/docs/api/RetryHandler.md index 6a932da8bdc..6dbc5077d02 100644 --- a/docs/docs/api/RetryHandler.md +++ b/docs/docs/api/RetryHandler.md @@ -35,6 +35,12 @@ Extends: [`Dispatch.DispatchOptions`](Dispatcher.md#parameter-dispatchoptions). - `state`: `RetryState` - Current retry state. It can be mutated. - `opts`: `Dispatch.DispatchOptions & RetryOptions` - Options passed to the retry handler. +**`RetryState`** + +It represents the retry state for a given request. + +- `counter`: `number` - Current retry attempt. + ### Parameter `RetryHandlers` - **dispatch** `(options: Dispatch.DispatchOptions, handlers: Dispatch.DispatchHandlers) => Promise` (required) - Dispatch function to be called after every retry. diff --git a/lib/handler/retry-handler.js b/lib/handler/retry-handler.js index 15894470ff3..204025c769a 100644 --- a/lib/handler/retry-handler.js +++ b/lib/handler/retry-handler.js @@ -60,6 +60,7 @@ class RetryHandler { } this.retryCount = 0 + this.retryCountCheckpoint = 0 this.start = 0 this.end = null this.etag = null @@ -112,10 +113,7 @@ class RetryHandler { errorCodes, methods } = retryOptions - let { counter, currentTimeout } = state - - currentTimeout = - currentTimeout != null && currentTimeout > 0 ? currentTimeout : minTimeout + const { counter } = state // Any code that is not a Undici's originated and allowed to retry if ( @@ -160,9 +158,7 @@ class RetryHandler { const retryTimeout = retryAfterHeader > 0 ? Math.min(retryAfterHeader, maxTimeout) - : Math.min(currentTimeout * timeoutFactor ** counter, maxTimeout) - - state.currentTimeout = retryTimeout + : Math.min(minTimeout * timeoutFactor ** (counter - 1), maxTimeout) setTimeout(() => cb(null), retryTimeout) } @@ -310,10 +306,19 @@ class RetryHandler { return this.handler.onError(err) } + // We reconcile in case of a mix between network errors + // and server error response + if (this.retryCount - this.retryCountCheckpoint > 0) { + // We count the difference between the last checkpoint and the current retry count + this.retryCount = this.retryCountCheckpoint + (this.retryCount - this.retryCountCheckpoint) + } else { + this.retryCount += 1 + } + this.retryOpts.retry( err, { - state: { counter: this.retryCount++, currentTimeout: this.retryAfter }, + state: { counter: this.retryCount }, opts: { retryOptions: this.retryOpts, ...this.opts } }, onRetry.bind(this) @@ -335,6 +340,7 @@ class RetryHandler { } try { + this.retryCountCheckpoint = this.retryCount this.dispatch(this.opts, this) } catch (err) { this.handler.onError(err) diff --git a/test/retry-handler.js b/test/retry-handler.js index 2792f23c2b0..1cbeed56ef8 100644 --- a/test/retry-handler.js +++ b/test/retry-handler.js @@ -17,7 +17,105 @@ test('Should retry status code', async t => { const dispatchOptions = { retryOptions: { retry: (err, { state, opts }, done) => { - counter++ + ++counter + + if ( + err.statusCode === 500 || + err.message.includes('other side closed') + ) { + setTimeout(done, 500) + return + } + + return done(err) + } + }, + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + } + + server.on('request', (req, res) => { + switch (counter) { + case 0: + req.destroy() + return + case 1: + res.writeHead(500) + res.end('failed') + return + case 2: + res.writeHead(200) + res.end('hello world!') + return + default: + t.fail() + } + }) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + const handler = new RetryHandler(dispatchOptions, { + dispatch: client.dispatch.bind(client), + handler: { + onConnect () { + t.ok(true, 'pass') + }, + onBodySent () { + t.ok(true, 'pass') + }, + onHeaders (status, _rawHeaders, resume, _statusMessage) { + t.strictEqual(status, 200) + return true + }, + onData (chunk) { + chunks.push(chunk) + return true + }, + onComplete () { + t.strictEqual(Buffer.concat(chunks).toString('utf-8'), 'hello world!') + t.strictEqual(counter, 2) + }, + onError () { + t.fail() + } + } + }) + + after(async () => { + await client.close() + server.close() + + await once(server, 'close') + }) + + client.dispatch( + { + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + }, + handler + ) + }) + + await t.completed +}) + +test('Should account for network and response errors', async t => { + t = tspl(t, { plan: 4 }) + + let counter = 0 + const chunks = [] + const server = createServer() + const dispatchOptions = { + retryOptions: { + retry: (err, { state, opts }, done) => { + counter = state.counter if ( err.statusCode === 500 || diff --git a/types/retry-handler.d.ts b/types/retry-handler.d.ts index 0528eb44279..e44b207c221 100644 --- a/types/retry-handler.d.ts +++ b/types/retry-handler.d.ts @@ -12,7 +12,7 @@ declare class RetryHandler implements Dispatcher.DispatchHandlers { } declare namespace RetryHandler { - export type RetryState = { counter: number; currentTimeout: number }; + export type RetryState = { counter: number; }; export type RetryContext = { state: RetryState;