Skip to content

Commit

Permalink
net, http: allow setting socket connection timeout for http request
Browse files Browse the repository at this point in the history
This allows passing the socket connection timeout to http#request
such that it will be set before the socket is connecting

Fixes nodejs#7580
  • Loading branch information
reneweb committed Aug 31, 2016
1 parent fce4b98 commit 4341e4d
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 18 deletions.
2 changes: 2 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,8 @@ Options:
request when the `agent` option is not used. This can be used to avoid
creating a custom Agent class just to override the default `createConnection`
function. See [`agent.createConnection()`][] for more details.
- `timeout`: A number specifying the socket timeout in milliseconds.
This will set the timeout before the socket is connected.

The optional `callback` parameter will be added as a one time listener for
the [`'response'`][] event.
Expand Down
4 changes: 3 additions & 1 deletion doc/api/net.md
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,9 @@ A factory function, which returns a new [`net.Socket`][] and automatically
connects with the supplied `options`.

The options are passed to both the [`net.Socket`][] constructor and the
[`socket.connect`][] method.
[`socket.connect`][] method.

Passing `timeout` as an option will call [`socket.setTimeout()`][] after the socket is created, but before it is connecting.

The `connectListener` parameter will be added as a listener for the
[`'connect'`][] event once.
Expand Down
8 changes: 7 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function ClientRequest(options, cb) {
}

self.socketPath = options.socketPath;
self.timeout = options.timeout;

var method = self.method = (options.method || 'GET').toUpperCase();
if (!common._checkIsHttpToken(method)) {
Expand Down Expand Up @@ -134,7 +135,8 @@ function ClientRequest(options, cb) {
self._last = true;
self.shouldKeepAlive = false;
const optionsPath = {
path: self.socketPath
path: self.socketPath,
timeout: self.timeout
};
const newSocket = self.agent.createConnection(optionsPath, oncreate);
if (newSocket && !called) {
Expand Down Expand Up @@ -559,6 +561,10 @@ function tickOnSocket(req, socket) {
socket.on('data', socketOnData);
socket.on('end', socketOnEnd);
socket.on('close', socketCloseListener);

if (req.timeout) {
socket.once('timeout', () => req.emit('timeout'));
}
req.emit('socket', socket);
}

Expand Down
5 changes: 5 additions & 0 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ exports.connect = exports.createConnection = function() {
args = normalizeConnectArgs(args);
debug('createConnection', args);
var s = new Socket(args[0]);

if (args[0].timeout) {
s.setTimeout(args[0].timeout);
}

return Socket.prototype.connect.apply(s, args);
};

Expand Down
24 changes: 8 additions & 16 deletions test/parallel/test-http-client-timeout-event.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var http = require('http');
const common = require('../common');
const assert = require('assert');
const http = require('http');

var options = {
method: 'GET',
Expand All @@ -10,30 +10,22 @@ var options = {
path: '/'
};

var server = http.createServer(function(req, res) {
// this space intentionally left blank
});
var server = http.createServer();

server.listen(0, options.host, function() {
options.port = this.address().port;
var req = http.request(options, function(res) {
// this space intentionally left blank
});
var req = http.request(options);
req.on('error', function() {
// this space is intentionally left blank
});
req.on('close', function() {
server.close();
});
req.on('close', common.mustCall(() => server.close()));

var timeout_events = 0;
req.setTimeout(1);
req.on('timeout', function() {
timeout_events += 1;
});
req.on('timeout', common.mustCall(() => timeout_events += 1));
setTimeout(function() {
req.destroy();
assert.equal(timeout_events, 1);
assert.strictEqual(timeout_events, 1);
}, common.platformTimeout(100));
setTimeout(function() {
req.end();
Expand Down
33 changes: 33 additions & 0 deletions test/parallel/test-http-client-timeout-option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

var options = {
method: 'GET',
port: undefined,
host: '127.0.0.1',
path: '/',
timeout: 1
};

var server = http.createServer();

server.listen(0, options.host, function() {
options.port = this.address().port;
var req = http.request(options);
req.on('error', function() {
// this space is intentionally left blank
});
req.on('close', common.mustCall(() => server.close()));

var timeout_events = 0;
req.on('timeout', common.mustCall(() => timeout_events += 1));
setTimeout(function() {
req.destroy();
assert.strictEqual(timeout_events, 1);
}, common.platformTimeout(100));
setTimeout(function() {
req.end();
}, common.platformTimeout(10));
});

0 comments on commit 4341e4d

Please sign in to comment.