Skip to content

Commit b3445e4

Browse files
committed
fix: ac lost on cloned request (#4068)
1 parent fdafc2a commit b3445e4

File tree

2 files changed

+80
-2
lines changed

2 files changed

+80
-2
lines changed

lib/web/fetch/request.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const assert = require('node:assert')
2929
const { getMaxListeners, setMaxListeners, defaultMaxListeners } = require('node:events')
3030

3131
const kAbortController = Symbol('abortController')
32+
const kDependantAbortController = Symbol('dependantAbortController')
3233

3334
const requestFinalizer = new FinalizationRegistry(({ signal, abort }) => {
3435
signal.removeEventListener('abort', abort)
@@ -428,6 +429,12 @@ class Request {
428429
// See, https://github.com/nodejs/undici/issues/1926.
429430
this[kAbortController] = ac
430431

432+
// Keep ref to original ac while cloned request is alive.
433+
// See, https://github.com/nodejs/undici/issues/4068.
434+
if (webidl.is.Request(input) && input[kAbortController]) {
435+
this[kDependantAbortController] = input[kAbortController]
436+
}
437+
431438
const acRef = new WeakRef(ac)
432439
const abort = buildAbort(acRef)
433440

@@ -790,8 +797,19 @@ class Request {
790797
)
791798
}
792799

793-
// 4. Return clonedRequestObject.
794-
return fromInnerRequest(clonedRequest, this.#dispatcher, ac.signal, getHeadersGuard(this.#headers))
800+
// 4. Construct clonedRequestObject.
801+
const req = fromInnerRequest(clonedRequest, this.#dispatcher, ac.signal, getHeadersGuard(this.#headers))
802+
req[kAbortController] = ac
803+
804+
// 5. Keep reference of original controller to avoid GC
805+
// Keep ref to original ac while cloned request is alive.
806+
// See, https://github.com/nodejs/undici/issues/4068.
807+
if (this[kAbortController]) {
808+
req[kDependantAbortController] = this[kAbortController]
809+
}
810+
811+
// 6. Return cloned request
812+
return req
795813
}
796814

797815
[nodeUtil.inspect.custom] (depth, options) {

test/fetch/issue-4068.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict'
2+
3+
const { once } = require('node:events')
4+
const { test, describe } = require('node:test')
5+
const { fetch, Request } = require('../..')
6+
const { createServer } = require('node:http')
7+
8+
describe('issue 4068', async () => {
9+
test('abort signal for new request', async (t) => {
10+
t.plan(2)
11+
const server = createServer(() => {}).listen(0)
12+
let aborted = false
13+
14+
t.after(server.close.bind(server))
15+
16+
const ac = new AbortController()
17+
let req = new Request(`http://localhost:${server.address().port}`, {
18+
signal: ac.signal
19+
})
20+
21+
req.signal.addEventListener('abort', () => {
22+
aborted = true
23+
})
24+
25+
req = new Request(req)
26+
setTimeout(() => {
27+
global.gc()
28+
ac.abort()
29+
})
30+
await once(server, 'listening')
31+
await t.assert.rejects(fetch(req))
32+
t.assert.ok(aborted)
33+
})
34+
35+
test('abort signal for cloned request', async (t) => {
36+
t.plan(2)
37+
const server = createServer(() => {}).listen(0)
38+
let aborted = false
39+
40+
t.after(server.close.bind(server))
41+
42+
const ac = new AbortController()
43+
let req = new Request(`http://localhost:${server.address().port}`, {
44+
signal: ac.signal
45+
})
46+
47+
req.signal.addEventListener('abort', () => {
48+
aborted = true
49+
})
50+
51+
req = req.clone()
52+
setTimeout(() => {
53+
global.gc()
54+
ac.abort()
55+
})
56+
await once(server, 'listening')
57+
await t.assert.rejects(fetch(req))
58+
t.assert.ok(aborted)
59+
})
60+
})

0 commit comments

Comments
 (0)