-
-
Notifications
You must be signed in to change notification settings - Fork 686
Description
Bug Description
The removal of async from Client.connect also removed a try/catch around the call to client[kConnector](...), which used to catch errors thrown by socket = tls.connect(...) in lib/core/connect.js. The catch in Client.connect would then perform clean up work, now handled by handleConnectError and a call to client[kResume](). Without the try/catch, this no longer occurs, and as a result the Client gets into a bad state wherein additional calls to Client.request will hang indefinitely.
Reproducible By
import { Client } from 'undici'
const client = new Client('https://example.com', {
connect: {
key: 'bad key',
cert: 'bad cert',
}
})
try {
await client.request({
method: 'GET',
path: '/',
})
} catch (err) {
console.log(
'Got the thrown error bubbled up from tls.connect, but the client is now in a bad state...',
err,
)
}
try {
await client.request({
method: 'GET',
path: '/',
})
} catch {
// irrelevant
}
console.log('We will never get here, because request will hang indefinitely...')Which, when run, will yield:
Warning: Detected unsettled top-level await at test.mjs:23
await client.request({
^Expected Behavior
The second call in the example above should throw the same error as the first call, with another (bad) tls.connect(...) attempt being made.
Logs & Screenshots
n/a
Environment
MacOS v15.7.1, Node v22.20.0
Additional context
Wrapping the call to client[kConnector](...) in a try/catch with the same cleanup as the err handling in its callback seems to fix the issue, i.e.
try {
client[kConnector](...)
} catch (err) {
handleConnectError(client, err, { host, hostname, protocol, port })
client[kResume]()
}This is the commit where the problem started occurring: 56d0cac