Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throw on retry when payload is consume by downstream #3389

Merged
merged 3 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/handler/retry-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class RetryHandler {
this.end = null
this.etag = null
this.resume = null
this.consumed = false

// Handle possible onConnect duplication
this.handler.onConnect(reason => {
Expand Down Expand Up @@ -193,6 +194,19 @@ class RetryHandler {
this.resume = null

if (statusCode !== 206) {
climba03003 marked this conversation as resolved.
Show resolved Hide resolved
// Abort when status code is not partial content
// and payload is already consumed because downstream
// will concatenate response from two request wrongly.
climba03003 marked this conversation as resolved.
Show resolved Hide resolved
if (this.consumed) {
this.abort(
new RequestRetryError('unsupported retry when payload is consumed', statusCode, {
climba03003 marked this conversation as resolved.
Show resolved Hide resolved
headers,
data: { count: this.retryCount }
})
)
return false
}

return true
}

Expand Down Expand Up @@ -294,6 +308,7 @@ class RetryHandler {
}

onData (chunk) {
this.consumed = true
climba03003 marked this conversation as resolved.
Show resolved Hide resolved
this.start += chunk.length

return this.handler.onData(chunk)
Expand Down
57 changes: 57 additions & 0 deletions test/issue-3356.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict'

const { tspl } = require('@matteo.collina/tspl')
const { test, after } = require('node:test')
const { createServer } = require('node:http')
const { once } = require('node:events')

const { fetch, Agent, RetryAgent } = require('..')

test('https://github.com/nodejs/undici/issues/3356', async (t) => {
t = tspl(t, { plan: 3 })

let shouldRetry = true
const server = createServer()
server.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' })
if (shouldRetry) {
shouldRetry = false

res.flushHeaders()
res.write('h')
setTimeout(() => { res.end('ello world!') }, 100)
} else {
res.end('hello world!')
}
})

server.listen(0)

await once(server, 'listening')

after(async () => {
server.close()

await once(server, 'close')
})

const agent = new RetryAgent(new Agent({ bodyTimeout: 50 }), {
errorCodes: ['UND_ERR_BODY_TIMEOUT']
})

const response = await fetch(`http://localhost:${server.address().port}`, {
dispatcher: agent
})
setTimeout(async () => {
try {
t.equal(response.status, 200)
// consume response
await response.text()
} catch (err) {
t.equal(err.name, 'TypeError')
t.equal(err.cause.code, 'UND_ERR_REQ_RETRY')
}
}, 200)

await t.completed
})
Loading