Skip to content

Commit

Permalink
http: set socket timeout when socket connects
Browse files Browse the repository at this point in the history
`request.setTimeout()` calls `socket.setTimeout()` as soon as a socket
is assigned to the request. This makes the `timeout` event to be
emitted on the request even if the underlying socket never connects.

This commit makes `socket.setTimeout()` to be called only when the
underlying socket is connected.

PR-URL: nodejs/node#8895
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
lpinca authored and Stephen Belanger committed Sep 21, 2017
1 parent 05a6ec2 commit 3cf1778
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,9 @@ ClientRequest.prototype.setTimeout = function setTimeout(msecs, callback) {
}

this.once('socket', function(sock) {
sock.setTimeout(msecs, emitTimeout);
sock.once('connect', function() {
sock.setTimeout(msecs, emitTimeout);
});
});

return this;
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-http-client-timeout-on-connect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

const server = http.createServer((req, res) => {
// This space is intentionally left blank.
});

server.listen(0, common.localhostIPv4, common.mustCall(() => {
const port = server.address().port;
const req = http.get(`http://${common.localhostIPv4}:${port}`);

req.setTimeout(1);
req.on('socket', common.mustCall((socket) => {
assert.strictEqual(socket._idleTimeout, undefined);
socket.on('connect', common.mustCall(() => {
assert.strictEqual(socket._idleTimeout, 1);
}));
}));
req.on('timeout', common.mustCall(() => req.abort()));
req.on('error', common.mustCall((err) => {
assert.strictEqual('socket hang up', err.message);
server.close();
}));
}));

0 comments on commit 3cf1778

Please sign in to comment.