http.Agent keepAlive connection pooling broken

[mweberxyz]

Version

v20.12.0

Platform

Linux ip-xxx 6.5.0-1014-aws #14~22.04.1-Ubuntu SMP Thu Feb 15 15:27:06 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

http

What steps will reproduce the bug?

Create an http.Agent with keepAlive: true, make a request to a given host, after that request succeeds, make a second request to the same host, after that request succeeds, observe open sockets.

test.js:

const http = require("http");

http.globalAgent = new http.Agent({ keepAlive: true });

const host = "www.neverssl.com"
const port = 80
const method = 'GET'

http.get("http://www.neverssl.com", function() {
  console.log("1 done")
  setTimeout(function () {
    http.get("http://www.neverssl.com", function() {
      console.log("2 done")
      process._getActiveHandles().forEach(function(h, idx) {
        if(h.localAddress) {
          console.log({
            idx: idx,
            destroyed: h.destroyed,
            localPort: h.localPort,
            remoteAddress: h.remoteAddress,
            remotePort: h.remotePort
          })
        }
      })
    });
  }, 1000)
});

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior? Why is that the expected behavior?

Only one socket is open at the conclusion of the second request. The second request should reuse the first socket due to keepAlive: true being set on the agent.

What do you see instead?

Two sockets are open at the conclusion of the second request

Additional information

This appears to be a regression some time during node v10:

for i in 20 10 9 8; do nvm use $i; node test.js; echo "------------------"; done:

Now using node v20.12.0 (npm v10.5.0)
1 done
2 done
{
  idx: 0,
  destroyed: false,
  localPort: 37328,
  remoteAddress: '34.223.124.45',
  remotePort: 80
}
{
  idx: 3,
  destroyed: false,
  localPort: 49906,
  remoteAddress: '34.223.124.45',
  remotePort: 80
}
------------------
Now using node v10.24.1 (npm v6.14.12)
1 done
2 done
{ idx: 2,
  destroyed: false,
  localPort: 49916,
  remoteAddress: '34.223.124.45',
  remotePort: 80 }
{ idx: 3,
  destroyed: false,
  localPort: 49922,
  remoteAddress: '34.223.124.45',
  remotePort: 80 }
------------------
Now using node v9.11.2 (npm v5.6.0)
1 done
2 done
{ idx: 2,
  destroyed: false,
  localPort: 48010,
  remoteAddress: '34.223.124.45',
  remotePort: 80 }
------------------
Now using node v8.17.0 (npm v6.13.4)
1 done
2 done
{ idx: 2,
  destroyed: false,
  localPort: 48026,
  remoteAddress: '34.223.124.45',
  remotePort: 80 }
------------------

[isker]

The behavior you’re reporting makes sense to me as you’re not consuming the response body of the first request (the first argument to the callback, which you are not using) before making the second.

[Malikrehman00107]

const http = require("http");

http.globalAgent = new http.Agent({ keepAlive: true });

const host = "www.neverssl.com";
const port = 80;
const method = 'GET';

http.get("http://www.neverssl.com", function(response1) {
  console.log("1 done");
  response1.resume(); // Consume the response body of the first request
  setTimeout(function () {
    http.get("http://www.neverssl.com", function(response2) {
      console.log("2 done");
      response2.resume(); // Consume the response body of the second request
      process._getActiveHandles().forEach(function(h, idx) {
        if(h.localAddress) {
          console.log({
            idx: idx,
            destroyed: h.destroyed,
            localPort: h.localPort,
            remoteAddress: h.remoteAddress,
            remotePort: h.remotePort
          });
        }
      });
    });
  }, 1000);
});

By calling the resume() method on the http.IncomingMessage objects (response1 and response2), you ensure that the response bodies are consumed, allowing the connections to be reused properly. This should result in only one socket being open at the conclusion of the second request.

[mweberxyz]

Thx @isker and @Malikrehman00107, my mistake.