diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js index 101acb60123..671ac199d46 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -449,65 +449,70 @@ function connect (client) { }) } - client[kConnector]({ - host, - hostname, - protocol, - port, - servername: client[kServerName], - localAddress: client[kLocalAddress] - }, (err, socket) => { - if (err) { - handleConnectError(client, err, { host, hostname, protocol, port }) - client[kResume]() - return - } + try { + client[kConnector]({ + host, + hostname, + protocol, + port, + servername: client[kServerName], + localAddress: client[kLocalAddress] + }, (err, socket) => { + if (err) { + handleConnectError(client, err, { host, hostname, protocol, port }) + client[kResume]() + return + } - if (client.destroyed) { - util.destroy(socket.on('error', noop), new ClientDestroyedError()) - client[kResume]() - return - } + if (client.destroyed) { + util.destroy(socket.on('error', noop), new ClientDestroyedError()) + client[kResume]() + return + } - assert(socket) + assert(socket) - try { - client[kHTTPContext] = socket.alpnProtocol === 'h2' - ? connectH2(client, socket) - : connectH1(client, socket) - } catch (err) { - socket.destroy().on('error', noop) - handleConnectError(client, err, { host, hostname, protocol, port }) - client[kResume]() - return - } + try { + client[kHTTPContext] = socket.alpnProtocol === 'h2' + ? connectH2(client, socket) + : connectH1(client, socket) + } catch (err) { + socket.destroy().on('error', noop) + handleConnectError(client, err, { host, hostname, protocol, port }) + client[kResume]() + return + } - client[kConnecting] = false - - socket[kCounter] = 0 - socket[kMaxRequests] = client[kMaxRequests] - socket[kClient] = client - socket[kError] = null - - if (channels.connected.hasSubscribers) { - channels.connected.publish({ - connectParams: { - host, - hostname, - protocol, - port, - version: client[kHTTPContext]?.version, - servername: client[kServerName], - localAddress: client[kLocalAddress] - }, - connector: client[kConnector], - socket - }) - } + client[kConnecting] = false + + socket[kCounter] = 0 + socket[kMaxRequests] = client[kMaxRequests] + socket[kClient] = client + socket[kError] = null + + if (channels.connected.hasSubscribers) { + channels.connected.publish({ + connectParams: { + host, + hostname, + protocol, + port, + version: client[kHTTPContext]?.version, + servername: client[kServerName], + localAddress: client[kLocalAddress] + }, + connector: client[kConnector], + socket + }) + } - client.emit('connect', client[kUrl], [client]) + client.emit('connect', client[kUrl], [client]) + client[kResume]() + }) + } catch (err) { + handleConnectError(client, err, { host, hostname, protocol, port }) client[kResume]() - }) + } } function handleConnectError (client, err, { host, hostname, protocol, port }) { diff --git a/test/node-test/client-connector-throw.js b/test/node-test/client-connector-throw.js new file mode 100644 index 00000000000..1556029f124 --- /dev/null +++ b/test/node-test/client-connector-throw.js @@ -0,0 +1,85 @@ +'use strict' + +const { test, after } = require('node:test') +const { tspl } = require('@matteo.collina/tspl') +const { Client } = require('../..') +const { createServer } = require('node:http') + +const { closeServerAsPromise } = require('../utils/node-http') + +test('client does not hang when connector throws synchronously', async (t) => { + const p = tspl(t, { plan: 4 }) + + // We need a server just so the URL resolves, but we'll never actually + // connect because our custom connector throws before that. + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.end() + }) + after(closeServerAsPromise(server)) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + connect: () => { + throw new Error('connector boom') + } + }) + after(() => client.destroy()) + + // First request should get the error from the throwing connector + client.request({ path: '/', method: 'GET' }, (err) => { + p.ok(err instanceof Error) + p.strictEqual(err.message, 'connector boom') + + // Second request should also get an error and not hang + client.request({ path: '/', method: 'GET' }, (err) => { + p.ok(err instanceof Error) + p.strictEqual(err.message, 'connector boom') + }) + }) + }) + + await p.completed +}) + +test('client recovers after connector stops throwing', async (t) => { + const p = tspl(t, { plan: 4 }) + + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.writeHead(200) + res.end('ok') + }) + after(closeServerAsPromise(server)) + + server.listen(0, () => { + let shouldThrow = true + const client = new Client(`http://localhost:${server.address().port}`, { + connect: (opts, cb) => { + if (shouldThrow) { + throw new Error('connector boom') + } + // Fall back to default connector behavior via tls/net + const net = require('node:net') + const socket = net.connect(opts.port, opts.hostname) + socket.on('connect', () => cb(null, socket)) + socket.on('error', (err) => cb(err, null)) + } + }) + after(() => client.destroy()) + + // First request fails because connector throws + client.request({ path: '/', method: 'GET' }, (err) => { + p.ok(err instanceof Error) + p.strictEqual(err.message, 'connector boom') + + // Now stop throwing so the next request can succeed + shouldThrow = false + + client.request({ path: '/', method: 'GET' }, (err, data) => { + p.ifError(err) + p.strictEqual(data.statusCode, 200) + }) + }) + }) + + await p.completed +})