Skip to content

Commit afd7194

Browse files
authored
fix: track origins by client map key in Agent teardown (#5537)
Signed-off-by: Emmanuel Yusufu Kimaswa <kimaswaemma36@gmail.com>
1 parent 08aae09 commit afd7194

2 files changed

Lines changed: 50 additions & 4 deletions

File tree

lib/dispatcher/agent.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ class Agent extends DispatcherBase {
107107
}
108108

109109
let hasOrigin = false
110-
for (const client of this[kClients].values()) {
111-
if (client[kUrl].origin === dispatcher[kUrl].origin) {
110+
for (const k of this[kClients].keys()) {
111+
if (k === origin || k === `${origin}#http1-only`) {
112112
hasOrigin = true
113113
break
114114
}
115115
}
116116

117117
if (!hasOrigin) {
118-
this[kOrigins].delete(dispatcher[kUrl].origin)
118+
this[kOrigins].delete(origin)
119119
}
120120
}
121121

test/agent-connection-management.js

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
const { test, describe } = require('node:test')
22
const assert = require('node:assert')
33
const { createServer } = require('node:http')
4-
const { request, Agent, Pool } = require('..')
4+
const { once } = require('node:events')
5+
const { request, Agent, Pool, ProxyAgent } = require('..')
6+
const { kClients } = require('../lib/core/symbols')
57

68
// https://github.com/nodejs/undici/issues/4424
79
describe('Agent should close inactive clients', () => {
@@ -154,3 +156,47 @@ describe('Agent should not close active clients', () => {
154156
assert.deepEqual(socketSequence.slice(3), ['2', '2'])
155157
})
156158
})
159+
160+
// https://github.com/nodejs/undici/issues/5529
161+
describe('Agent teardown of factory dispatchers without an internal url', () => {
162+
test('ProxyAgent forwarding plain http does not crash on teardown', async (t) => {
163+
// A minimal forward proxy: answers any absolute-form request itself.
164+
const proxy = createServer((req, res) => {
165+
res.setHeader('connection', 'close')
166+
res.end('ok')
167+
}).listen(0)
168+
169+
t.after(() => {
170+
proxy.closeAllConnections?.()
171+
proxy.close()
172+
})
173+
await once(proxy, 'listening')
174+
175+
const proxyAgent = new ProxyAgent(`http://localhost:${proxy.address().port}`)
176+
t.after(() => proxyAgent.close())
177+
178+
// A plain-http request registers an Http1ProxyWrapper, which has no kUrl,
179+
// in the inner Agent's client map.
180+
const { statusCode, body } = await request('http://target.example/', { dispatcher: proxyAgent })
181+
assert.equal(statusCode, 200)
182+
await body.text()
183+
184+
let innerAgent
185+
for (const sym of Object.getOwnPropertySymbols(proxyAgent)) {
186+
const value = proxyAgent[sym]
187+
if (value && value[kClients] instanceof Map && value[kClients].size > 0) {
188+
innerAgent = value
189+
break
190+
}
191+
}
192+
assert.ok(innerAgent, 'expected to find the inner Agent')
193+
const [wrapper] = innerAgent[kClients].values()
194+
195+
// The Agent subscribes to this event on every dispatcher its factory
196+
// returns. Before the fix this threw
197+
// "Cannot read properties of undefined (reading 'origin')".
198+
wrapper.emit('disconnect', 'http://target.example', [wrapper], new Error('closed'))
199+
200+
assert.equal(innerAgent[kClients].size, 0, 'expected the unused wrapper to be removed')
201+
})
202+
})

0 commit comments

Comments
 (0)