http: attach error handler to socket synchronously in onSocket

RajeshKumar11 · aduh95 · commit 435f3dd8e4f8 · 2026-02-19T11:14:04.000+01:00
Between onSocket and onSocketNT, the socket had no error handler, meaning any errors emitted during that window (e.g. from a blocklist check or custom lookup) would be unhandled even if the user had set up a request error handler. Fix this by attaching socketErrorListener synchronously in onSocket, setting socket._httpMessage so the listener can forward errors to the request. The _destroy path in onSocketNT is also guarded to prevent double-firing if socketErrorListener already emitted the error. Fixes: #48771 Refs: #61658 PR-URL: #61770 Refs: #48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git 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);
   }
 
@@ -910,7 +910,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);
@@ -949,8 +948,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);
 };
 
@@ -962,8 +968,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
@@ -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
@@ -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);
