Visitar URL original
fix(retry): settle exposed body on terminal failure (#5778) · nodejs/undici@2a91fc8 · GitHub
Skip to content

Commit 2a91fc8

Browse files
authored
fix(retry): settle exposed body on terminal failure (#5778)
Adapt the response-ownership guard from e905b5b to the v6 handler interface. Abort terminal attempts instead of replacing a response already exposed downstream, while preserving initial response forwarding. Add handler-level regression coverage for terminal errors and initial non-retryable responses.
1 parent ffc8aa0 commit 2a91fc8

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

‎lib/handler/retry-handler.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,10 @@ class RetryHandler {
205205
this.retryCount += 1
206206

207207
if (statusCode >= 300) {
208-
if (this.retryOpts.statusCodes.includes(statusCode) === false) {
208+
// Only expose a response if no earlier attempt has reached the caller.
209+
// Otherwise abort this attempt so the error settles the existing body
210+
// instead of replacing it with a new response.
211+
if (!this.headersSent && this.retryOpts.statusCodes.includes(statusCode) === false) {
209212
this.headersSent = true
210213
this.checkpointResponseEnd(headers, resume)
211214
return this.handler.onHeaders(

‎test/retry-handler-terminal.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
'use strict'
2+
3+
const assert = require('node:assert/strict')
4+
const { test } = require('node:test')
5+
const { RetryHandler } = require('..')
6+
7+
function noop () {}
8+
9+
for (const statusCode of [300, 404, 416]) {
10+
test(`RetryHandler settles an exposed response on terminal status ${statusCode}`, () => {
11+
const errors = []
12+
let aborts = 0
13+
const handler = new RetryHandler({ method: 'GET', path: '/' }, {
14+
dispatch () {
15+
assert.fail('a terminal status must not dispatch another attempt')
16+
},
17+
handler: {
18+
onConnect: noop,
19+
onHeaders () {
20+
assert.fail('an exposed response must not be replaced')
21+
},
22+
onError (err) {
23+
errors.push(err)
24+
}
25+
}
26+
})
27+
28+
// Model an attempt whose response is already owned by the caller.
29+
handler.headersSent = true
30+
handler.resume = noop
31+
handler.start = 1
32+
handler.end = 3
33+
handler.onConnect(err => {
34+
aborts++
35+
handler.onError(err)
36+
})
37+
38+
const rawHeaders = ['content-length', '0']
39+
assert.equal(handler.onHeaders(statusCode, rawHeaders, noop, 'Terminal'), false)
40+
assert.equal(aborts, 1)
41+
assert.equal(errors.length, 1)
42+
assert.equal(errors[0].code, 'UND_ERR_REQ_RETRY')
43+
assert.equal(errors[0].statusCode, statusCode)
44+
assert.deepEqual(errors[0].headers, { 'content-length': '0' })
45+
assert.equal(handler.start, 1)
46+
assert.equal(handler.end, 3)
47+
})
48+
49+
test(`RetryHandler preserves an initial non-retryable response with status ${statusCode}`, () => {
50+
const rawHeaders = ['content-length', '0']
51+
let responses = 0
52+
let completions = 0
53+
const resume = noop
54+
const handler = new RetryHandler({ method: 'GET', path: '/' }, {
55+
dispatch () {
56+
assert.fail('an initial non-retryable response must not be retried')
57+
},
58+
handler: {
59+
onConnect: noop,
60+
onHeaders (status, headers, receivedResume, statusMessage) {
61+
responses++
62+
assert.equal(status, statusCode)
63+
assert.equal(headers, rawHeaders)
64+
assert.equal(receivedResume, resume)
65+
assert.equal(statusMessage, 'Terminal')
66+
return true
67+
},
68+
onComplete () {
69+
completions++
70+
},
71+
onError (err) {
72+
assert.fail(err)
73+
}
74+
}
75+
})
76+
handler.onConnect(() => assert.fail('the initial response must not be aborted'))
77+
78+
assert.equal(handler.onHeaders(statusCode, rawHeaders, resume, 'Terminal'), true)
79+
handler.onComplete([])
80+
assert.equal(responses, 1)
81+
assert.equal(completions, 1)
82+
assert.equal(handler.headersSent, true)
83+
assert.equal(handler.end, -1)
84+
})
85+
}

0 commit comments

Comments
 (0)