http: set default timeout in agent keepSocketAlive

Previous location of setting the timeout would override
behaviour of custom HttpAgents' keepSocketAlive. Moving
it into the default keepSocketAlive allows it to
interoperate with custom agents.

Fixes: https://github.com/nodejs/node/issues/33111

PR-URL: https://github.com/nodejs/node/pull/33127
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>

lib/_http_agent.js

@@ -138,11 +138,6 @@ function Agent(options) {
     socket._httpMessage = null;
     this.removeSocket(socket, options);
 
-    const agentTimeout = this.options.timeout || 0;
-    if (socket.timeout !== agentTimeout) {
-      socket.setTimeout(agentTimeout);
-    }
-
     socket.once('error', freeSocketErrorListener);
     freeSockets.push(socket);
   });
@@ -402,6 +397,11 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
   socket.setKeepAlive(true, this.keepAliveMsecs);
   socket.unref();
 
+  const agentTimeout = this.options.timeout || 0;
+  if (socket.timeout !== agentTimeout) {
+    socket.setTimeout(agentTimeout);
+  }
+
   return true;
 };
 

test/parallel/test-http-agent-timeout.js

@@ -92,3 +92,43 @@ const http = require('http');
       }));
   }));
 }
+
+{
+  // Ensure custom keepSocketAlive timeout is respected
+
+  const CUSTOM_TIMEOUT = 60;
+  const AGENT_TIMEOUT = 50;
+
+  class CustomAgent extends http.Agent {
+    keepSocketAlive(socket) {
+      if (!super.keepSocketAlive(socket)) {
+        return false;
+      }
+
+      socket.setTimeout(CUSTOM_TIMEOUT);
+      return true;
+    }
+  }
+
+  const agent = new CustomAgent({ keepAlive: true, timeout: AGENT_TIMEOUT });
+
+  const server = http.createServer((req, res) => {
+    res.end();
+  });
+
+  server.listen(0, common.mustCall(() => {
+    http.get({ port: server.address().port, agent })
+      .on('response', common.mustCall((res) => {
+        const socket = res.socket;
+        assert(socket);
+        res.resume();
+        socket.on('free', common.mustCall(() => {
+          socket.on('timeout', common.mustCall(() => {
+            assert.strictEqual(socket.timeout, CUSTOM_TIMEOUT);
+            agent.destroy();
+            server.close();
+          }));
+        }));
+      }));
+  }));
+}