http: socket connection timeout for http request

This allows passing the socket connection timeout to http#request
such that it will be set before the socket is connecting

PR-URL: https://github.com/nodejs/node/pull/8101
Fixes: https://github.com/nodejs/node/issues/7580
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>

doc/api/http.md

@@ -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.

doc/api/net.md

@@ -765,7 +765,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.

lib/_http_client.js

@@ -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)) {
@@ -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) {
@@ -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);
 }
 

lib/net.js

@@ -66,6 +66,11 @@ exports.connect = exports.createConnection = function() {
   args = normalizeArgs(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);
 };
 

test/parallel/test-http-client-timeout-event.js

@@ -1,7 +1,7 @@
 'use strict';
-var common = require('../common');
+const common = require('../common');
-var assert = require('assert');
+const assert = require('assert');
-var http = require('http');
+const http = require('http');
 
 var options = {
   method: 'GET',
@@ -10,30 +10,22 @@ var options = {
   path: '/'
 };
 
-var server = http.createServer(function(req, res) {
+var server = http.createServer();
-  // this space intentionally left blank
-});
 
 server.listen(0, options.host, function() {
   options.port = this.address().port;
-  var req = http.request(options, function(res) {
+  var req = http.request(options);
-    // this space intentionally left blank
-  });
   req.on('error', function() {
     // this space is intentionally left blank
   });
-  req.on('close', function() {
+  req.on('close', common.mustCall(() => server.close()));
-    server.close();
-  });
 
   var timeout_events = 0;
   req.setTimeout(1);
-  req.on('timeout', function() {
+  req.on('timeout', common.mustCall(() => timeout_events += 1));
-    timeout_events += 1;
-  });
   setTimeout(function() {
     req.destroy();
-    assert.equal(timeout_events, 1);
+    assert.strictEqual(timeout_events, 1);
   }, common.platformTimeout(100));
   setTimeout(function() {
     req.end();

test/parallel/test-http-client-timeout-option.js

@@ -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));
+});