From fcc49dbeeb3453a8f51fde0f66dfd8a1e8079f4d Mon Sep 17 00:00:00 2001
From: anilhelvaci <anilhelvaci96@gmail.com>
Date: Fri, 6 Sep 2024 17:47:15 +0300
Subject: [PATCH] net: defer self.destroy calls to nextTick

Wrote tests for the suggested fix in https://github.com/nodejs/node/issues/48771#issuecomment-1661858958

Fixes: https://github.com/nodejs/node/issues/48771
---
 lib/net.js                                    |  8 +-
 .../test-http-client-immediate-error.js       | 95 +++++++++++++++----
 2 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index a391e9da30f861..dae3f61767969d 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -1066,7 +1066,7 @@ function internalConnect(
     err = checkBindError(err, localPort, self._handle);
     if (err) {
       const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort);
-      self.destroy(ex);
+      process.nextTick(() => self.destroy(ex));
       return;
     }
   }
@@ -1108,7 +1108,7 @@ function internalConnect(
     }
 
     const ex = new ExceptionWithHostPort(err, 'connect', address, port, details);
-    self.destroy(ex);
+    process.nextTick(() => self.destroy(ex));
   } else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
     startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
   }
@@ -1127,11 +1127,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) {
-      self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT());
+      process.nextTick(() => self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT()));
       return;
     }
 
-    self.destroy(new NodeAggregateError(context.errors));
+    process.nextTick(() => self.destroy(new NodeAggregateError(context.errors)));
     return;
   }
 
diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js
index ff29ad72bd8585..1533be5d4f2d31 100644
--- a/test/parallel/test-http-client-immediate-error.js
+++ b/test/parallel/test-http-client-immediate-error.js
@@ -9,36 +9,95 @@ const assert = require('assert');
 const net = require('net');
 const http = require('http');
 const { internalBinding } = require('internal/test/binding');
-const { UV_ENETUNREACH } = internalBinding('uv');
+const { UV_ENETUNREACH, UV_EADDRINUSE } = internalBinding('uv');
 const {
   newAsyncId,
   symbols: { async_id_symbol }
 } = require('internal/async_hooks');
 
-const agent = new http.Agent();
-agent.createConnection = common.mustCall((cfg) => {
-  const sock = new net.Socket();
+const config = {
+  host: 'http://example.com',
+  // We need to specify 'family' in both items of the array so 'internalConnectMultiple' is invoked
+  connectMultiple: [{ address: '192.4.4.4', family: 4 }, { address: '200::1', family: 6 }],
+  connectSolo: { address: '192.4.4.4', family: 4 },
+};
 
-  // Fake the handle so we can enforce returning an immediate error
-  sock._handle = {
-    connect: common.mustCall((req, addr, port) => {
-      return UV_ENETUNREACH;
-    }),
-    readStart() {},
-    close() {}
-  };
+function agentFactory(handle, count) {
+  const agent = new http.Agent();
+  agent.createConnection = common.mustCall((cfg) => {
+    const sock = new net.Socket();
 
-  // Simulate just enough socket handle initialization
-  sock[async_id_symbol] = newAsyncId();
+    // Fake the handle so we can enforce returning an immediate error
+    sock._handle = handle;
 
-  sock.connect(cfg);
-  return sock;
-});
+    // Simulate just enough socket handle initialization
+    sock[async_id_symbol] = newAsyncId();
+
+    sock.connect(cfg);
+    return sock;
+  }, count);
+
+  return agent;
+};
+
+const handleWithoutBind = {
+  connect: common.mustCall((req, addr, port) => {
+    return UV_ENETUNREACH;
+  }, 3), // Because two tests will use this
+  readStart() { },
+  close() { },
+};
+
+const handleWithBind = {
+  readStart() { },
+  close() { },
+  bind: common.mustCall(() => UV_EADDRINUSE, 2), // Because two tests will use this handle
+};
+
+const agentWithoutBind = agentFactory(handleWithoutBind, 3);
+const agentWithBind = agentFactory(handleWithBind, 2);
 
 http.get({
   host: '127.0.0.1',
   port: 1,
-  agent
+  agent: agentWithoutBind,
 }).on('error', common.mustCall((err) => {
   assert.strictEqual(err.code, 'ENETUNREACH');
 }));
+
+http.request(config.host, {
+  agent: agentWithoutBind,
+  lookup(_0, _1, cb) {
+    cb(null, config.connectMultiple);
+  },
+}).on('error', common.mustCall((err) => {
+  assert.strictEqual(err.code, 'ENETUNREACH');
+}));
+
+http.request(config.host, {
+  agent: agentWithoutBind,
+  lookup(_0, _1, cb) {
+    cb(null, config.connectSolo.address, config.connectSolo.family);
+  },
+  family: 4,
+}).on('error', common.mustCall((err) => {
+  assert.strictEqual(err.code, 'ENETUNREACH');
+}));
+
+http.request(config.host, {
+  agent: agentWithBind,
+  family: 4, // We specify 'family' so 'internalConnect' is invoked
+  localPort: 2222 // Required to trigger _handle.bind()
+}).on('error', common.mustCall((err) => {
+  assert.strictEqual(err.code, 'EADDRINUSE');
+}));
+
+http.request(config.host, {
+  agent: agentWithBind,
+  lookup(_0, _1, cb) {
+    cb(null, config.connectMultiple);
+  },
+  localPort: 2222, // Required to trigger _handle.bind()
+}).on('error', common.mustCall((err) => {
+  assert.strictEqual(err.code, 'EADDRINUSE');
+}));