Skip to content

Commit

Permalink
http: set default timeout in agent keepSocketAlive
Browse files Browse the repository at this point in the history
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: #33111

PR-URL: #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>
  • Loading branch information
omsmith authored and targos committed May 4, 2020
1 parent e276524 commit 451993e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/_http_agent.js
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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;
};

Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-http-agent-timeout.js
Expand Up @@ -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();
}));
}));
}));
}));
}

0 comments on commit 451993e

Please sign in to comment.