Skip to content

Commit db95b63

Browse files
MzUgMcrysmags
authored andcommitted
Expose HTTP errors that are not meant to be retried (nodejs#2496)
* test: failing test to show http response not passed through * fix: pass through requests that do not expect a retry * refactor: cleanup
1 parent cafa798 commit db95b63

File tree

2 files changed

+85
-7
lines changed

2 files changed

+85
-7
lines changed

lib/handler/RetryHandler.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,22 @@ class RetryHandler {
172172
this.retryCount += 1
173173

174174
if (statusCode >= 300) {
175-
this.abort(
176-
new RequestRetryError('Request failed', statusCode, {
177-
headers,
178-
count: this.retryCount
179-
})
180-
)
181-
return false
175+
if (this.retryOpts.statusCodes.includes(statusCode) === false) {
176+
return this.handler.onHeaders(
177+
statusCode,
178+
rawHeaders,
179+
resume,
180+
statusMessage
181+
)
182+
} else {
183+
this.abort(
184+
new RequestRetryError('Request failed', statusCode, {
185+
headers,
186+
count: this.retryCount
187+
})
188+
)
189+
return false
190+
}
182191
}
183192

184193
// Checkpoint for resume from where we left it

test/retry-handler.js

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,3 +620,72 @@ tap.test('retrying a request with a body', t => {
620620
)
621621
})
622622
})
623+
624+
tap.test('should not error if request is not meant to be retried', t => {
625+
const server = createServer()
626+
server.on('request', (req, res) => {
627+
res.writeHead(400)
628+
res.end('Bad request')
629+
})
630+
631+
t.plan(3)
632+
633+
const dispatchOptions = {
634+
retryOptions: {
635+
method: 'GET',
636+
path: '/',
637+
headers: {
638+
'content-type': 'application/json'
639+
}
640+
}
641+
}
642+
643+
server.listen(0, () => {
644+
const client = new Client(`http://localhost:${server.address().port}`)
645+
const chunks = []
646+
const handler = new RetryHandler(dispatchOptions, {
647+
dispatch: client.dispatch.bind(client),
648+
handler: {
649+
onConnect () {
650+
t.pass()
651+
},
652+
onBodySent () {
653+
t.pass()
654+
},
655+
onHeaders (status, _rawHeaders, resume, _statusMessage) {
656+
t.equal(status, 400)
657+
return true
658+
},
659+
onData (chunk) {
660+
chunks.push(chunk)
661+
return true
662+
},
663+
onComplete () {
664+
t.equal(Buffer.concat(chunks).toString('utf-8'), 'Bad request')
665+
},
666+
onError (err) {
667+
console.log({ err })
668+
t.fail()
669+
}
670+
}
671+
})
672+
673+
t.teardown(async () => {
674+
await client.close()
675+
server.close()
676+
677+
await once(server, 'close')
678+
})
679+
680+
client.dispatch(
681+
{
682+
method: 'GET',
683+
path: '/',
684+
headers: {
685+
'content-type': 'application/json'
686+
}
687+
},
688+
handler
689+
)
690+
})
691+
})

0 commit comments

Comments
 (0)