Skip to content

Commit 44c23e5

Browse files
authored
fix: fix wrong stream canceled up after cloning (v6) (#4414)
* Add failing test repro * fix: register correct stream with finalization registry * fix: node21 * port changes * remove duplicate test * be defensive with finalization registry
1 parent da0e823 commit 44c23e5

File tree

3 files changed

+44
-6
lines changed

3 files changed

+44
-6
lines changed

lib/web/fetch/body.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,6 @@ function cloneBody (instance, body) {
296296
// 1. Let « out1, out2 » be the result of teeing body’s stream.
297297
const [out1, out2] = body.stream.tee()
298298

299-
if (hasFinalizationRegistry) {
300-
streamRegistry.register(instance, new WeakRef(out1))
301-
}
302-
303299
// 2. Set body’s stream to out1.
304300
body.stream = out1
305301

lib/web/fetch/response.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,11 @@ class Response {
240240
// 2. Let clonedResponse be the result of cloning this’s response.
241241
const clonedResponse = cloneResponse(this[kState])
242242

243+
// Note: To re-register because of a new stream.
244+
if (hasFinalizationRegistry && this[kState].body?.stream) {
245+
streamRegistry.register(this, new WeakRef(this[kState].body.stream))
246+
}
247+
243248
// 3. Return the result of creating a Response object, given
244249
// clonedResponse, this’s headers’s guard, and this’s relevant Realm.
245250
return fromInnerResponse(clonedResponse, getHeadersGuard(this[kHeaders]))

test/fetch/fire-and-forget.js

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,54 @@
11
'use strict'
22

33
const { randomFillSync } = require('node:crypto')
4-
const { setTimeout: sleep } = require('timers/promises')
4+
const { setTimeout: sleep, setImmediate: nextTick } = require('node:timers/promises')
55
const { test } = require('node:test')
6-
const { fetch, Agent, setGlobalDispatcher } = require('../..')
6+
const { fetch, Request, Response, Agent, setGlobalDispatcher } = require('../..')
77
const { createServer } = require('node:http')
88
const { closeServerAsPromise } = require('../utils/node-http')
99

1010
const blob = randomFillSync(new Uint8Array(1024 * 512))
1111

12+
const hasGC = typeof global.gc !== 'undefined'
13+
1214
// Enable when/if FinalizationRegistry in Node.js 18 becomes stable again
1315
const isNode18 = process.version.startsWith('v18')
1416

17+
// https://github.com/nodejs/undici/issues/4150
18+
test('test finalizer cloned request', async () => {
19+
if (!hasGC) {
20+
throw new Error('gc is not available. Run with \'--expose-gc\'.')
21+
}
22+
23+
const request = new Request('http://localhost', { method: 'POST', body: 'Hello' })
24+
25+
request.clone()
26+
27+
await nextTick()
28+
// eslint-disable-next-line no-undef
29+
gc()
30+
31+
await nextTick()
32+
await request.arrayBuffer() // check consume body
33+
})
34+
35+
test('test finalizer cloned response', async () => {
36+
if (!hasGC) {
37+
throw new Error('gc is not available. Run with \'--expose-gc\'.')
38+
}
39+
40+
const response = new Response('Hello')
41+
42+
response.clone()
43+
44+
await nextTick()
45+
// eslint-disable-next-line no-undef
46+
gc()
47+
48+
await nextTick()
49+
await response.arrayBuffer() // check consume body
50+
})
51+
1552
test('does not need the body to be consumed to continue', { timeout: 180_000, skip: isNode18 }, async (t) => {
1653
const agent = new Agent({
1754
keepAliveMaxTimeout: 10,

0 commit comments

Comments
 (0)