diff --git a/lib/_http_client.js b/lib/_http_client.js index ee4f47be64ab3c..99998b30b8946c 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -571,7 +571,7 @@ function socketErrorListener(err) { if (req) { // For Safety. Some additional errors might fire later on // and we need to make sure we don't double-fire the error event. - req.socket._hadError = true; + socket._hadError = true; emitErrorEvent(req, err); } @@ -906,7 +906,6 @@ function tickOnSocket(req, socket) { parser.joinDuplicateHeaders = req.joinDuplicateHeaders; parser.onIncoming = parserOnIncomingClient; - socket.on('error', socketErrorListener); socket.on('data', socketOnData); socket.on('end', socketOnEnd); socket.on('close', socketCloseListener); @@ -945,8 +944,15 @@ function listenSocketTimeout(req) { } ClientRequest.prototype.onSocket = function onSocket(socket, err) { - // TODO(ronag): Between here and onSocketNT the socket - // has no 'error' handler. + // Attach the error listener synchronously so that any errors emitted on + // the socket before onSocketNT runs (e.g. from a blocklist check or other + // next-tick error) are forwarded to the request and can be caught by the + // user's error handler. socketErrorListener requires socket._httpMessage + // to be set so we set it here too. + if (socket && !err) { + socket._httpMessage = this; + socket.on('error', socketErrorListener); + } process.nextTick(onSocketNT, this, socket, err); }; @@ -958,8 +964,10 @@ function onSocketNT(req, socket, err) { if (!req.aborted && !err) { err = new ConnResetException('socket hang up'); } - // ERR_PROXY_TUNNEL is handled by the proxying logic - if (err && err.code !== 'ERR_PROXY_TUNNEL') { + // ERR_PROXY_TUNNEL is handled by the proxying logic. + // Skip if the error was already emitted by the early socketErrorListener. + if (err && err.code !== 'ERR_PROXY_TUNNEL' && + !socket?._hadError) { emitErrorEvent(req, err); } req._closed = true; diff --git a/lib/net.js b/lib/net.js index cbde238ba0a2ce..e22ef4bfc4bff0 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1125,7 +1125,7 @@ function internalConnect( err = checkBindError(err, localPort, self._handle); if (err) { const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort); - process.nextTick(emitErrorAndDestroy, self, ex); + self.destroy(ex); return; } } @@ -1135,7 +1135,7 @@ function internalConnect( if (addressType === 6 || addressType === 4) { if (self.blockList?.check(address, `ipv${addressType}`)) { - process.nextTick(emitErrorAndDestroy, self, new ERR_IP_BLOCKED(address)); + self.destroy(new ERR_IP_BLOCKED(address)); return; } const req = new TCPConnectWrap(); @@ -1167,20 +1167,12 @@ function internalConnect( } const ex = new ExceptionWithHostPort(err, 'connect', address, port, details); - process.nextTick(emitErrorAndDestroy, self, ex); + self.destroy(ex); } else if ((addressType === 6 || addressType === 4) && hasObserver('net')) { startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } }); } } -// Helper function to defer socket destruction to the next tick. -// This ensures that error handlers have a chance to be set up -// before the error is emitted, particularly important when using -// http.request with a custom lookup function. -function emitErrorAndDestroy(self, err) { - self.destroy(err); -} - function internalConnectMultiple(context, canceled) { clearTimeout(context[kTimeout]); @@ -1194,11 +1186,11 @@ function internalConnectMultiple(context, canceled) { // All connections have been tried without success, destroy with error if (canceled || context.current === context.addresses.length) { if (context.errors.length === 0) { - process.nextTick(emitErrorAndDestroy, self, new ERR_SOCKET_CONNECTION_TIMEOUT()); + self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT()); return; } - process.nextTick(emitErrorAndDestroy, self, new NodeAggregateError(context.errors)); + self.destroy(new NodeAggregateError(context.errors)); return; } diff --git a/test/parallel/test-http-request-lookup-error-catchable.js b/test/parallel/test-http-request-lookup-error-catchable.js index 905f841c77c096..242f6ad45a553f 100644 --- a/test/parallel/test-http-request-lookup-error-catchable.js +++ b/test/parallel/test-http-request-lookup-error-catchable.js @@ -13,8 +13,8 @@ const net = require('net'); // 2. The lookup returns an IP that triggers a synchronous error (e.g., blockList) // 3. The error is emitted before http's error handler is set up (via nextTick) // -// The fix defers socket.destroy() calls in internalConnect to the next tick, -// giving http.request() time to set up its error handlers. +// The fix attaches socketErrorListener synchronously in onSocket so that +// socket errors are forwarded to the request before onSocketNT runs. const blockList = new net.BlockList(); blockList.addAddress(common.localhostIPv4);