Skip to content

Commit ed15e98

Browse files
authoredNov 30, 2023
Make call to onBodySent conditional in RetryHandler (#2478)
* failing test RequestHandler does not have `onBodySent` but `RetryHandler` always sends to `onBodySent` which causes a problem for users who use `RetryHandler` via `request()` * is this a fix? * refactor: clean up test case
1 parent 19c69a0 commit ed15e98

File tree

3 files changed

+80
-1
lines changed

3 files changed

+80
-1
lines changed
 

‎lib/api/api-request.js

+1
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,4 @@ function request (opts, callback) {
177177
}
178178

179179
module.exports = request
180+
module.exports.RequestHandler = RequestHandler

‎lib/handler/RetryHandler.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class RetryHandler {
9595
}
9696

9797
onBodySent (chunk) {
98-
return this.handler.onBodySent(chunk)
98+
if (this.handler.onBodySent) return this.handler.onBodySent(chunk)
9999
}
100100

101101
static [kRetryHandlerDefaultRetry] (err, { state, opts }, cb) {

‎test/retry-handler.js

+78
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { once } = require('node:events')
55
const tap = require('tap')
66

77
const { RetryHandler, Client } = require('..')
8+
const { RequestHandler } = require('../lib/api/api-request')
89

910
tap.test('Should retry status code', t => {
1011
let counter = 0
@@ -542,3 +543,80 @@ tap.test('Should handle 206 partial content - bad-etag', t => {
542543
})
543544
})
544545
})
546+
547+
tap.test('retrying a request with a body', t => {
548+
let counter = 0
549+
const server = createServer()
550+
const dispatchOptions = {
551+
retryOptions: {
552+
retry: (err, { state, opts }, done) => {
553+
counter++
554+
555+
if (
556+
err.statusCode === 500 ||
557+
err.message.includes('other side closed')
558+
) {
559+
setTimeout(done, 500)
560+
return
561+
}
562+
563+
return done(err)
564+
}
565+
},
566+
method: 'POST',
567+
path: '/',
568+
headers: {
569+
'content-type': 'application/json'
570+
},
571+
body: JSON.stringify({ hello: 'world' })
572+
}
573+
574+
t.plan(1)
575+
576+
server.on('request', (req, res) => {
577+
switch (counter) {
578+
case 0:
579+
req.destroy()
580+
return
581+
case 1:
582+
res.writeHead(500)
583+
res.end('failed')
584+
return
585+
case 2:
586+
res.writeHead(200)
587+
res.end('hello world!')
588+
return
589+
default:
590+
t.fail()
591+
}
592+
})
593+
594+
server.listen(0, () => {
595+
const client = new Client(`http://localhost:${server.address().port}`)
596+
const handler = new RetryHandler(dispatchOptions, {
597+
dispatch: client.dispatch.bind(client),
598+
handler: new RequestHandler(dispatchOptions, (err, data) => {
599+
t.error(err)
600+
})
601+
})
602+
603+
t.teardown(async () => {
604+
await client.close()
605+
server.close()
606+
607+
await once(server, 'close')
608+
})
609+
610+
client.dispatch(
611+
{
612+
method: 'POST',
613+
path: '/',
614+
headers: {
615+
'content-type': 'application/json'
616+
},
617+
body: JSON.stringify({ hello: 'world' })
618+
},
619+
handler
620+
)
621+
})
622+
})

0 commit comments

Comments
 (0)
Please sign in to comment.