Add ability to detect connect timeouts

It's valuable to be able to detect whether a timeout occurred on the `connect`
syscall or on the `write`/`read` syscall of an HTTP request - the former
implies that your client can't reach the server and the latter implies the
server might not have responded.

For backwards compatibility, and standardization with Unix errno codes, the
`code` property of connect timeout errors must remain as 'ETIMEDOUT'. Instead
of changing this value, add a new `connect` property on timeout errors, which
is set to true if the error occurred on the `connect` attempt. Clients have
to opt in to checking it - in the common read timeout case, it will be set to
`false`.

Updates the documentation around each timeout to clarify what is going on, and
what situations can trigger each timeout.

Updates the README with more accurate documentation about timeout times
in the wild. I tried `time curl http://10.255.255.1` and `time curl
http://google.com:81` on four different Linux machines (Heroku, boot2docker,
Centos on DigitalOcean, a VPS) and the timeout times ranged from 60 seconds to
120 seconds, much longer than the 20 seconds previously suggested.

Updates the README with information on how to detect connection timeout
failures.

README.md

@@ -784,9 +784,12 @@ The first argument can be either a `url` or an `options` object. The only requir
     with your pool options or create the pool object with the `maxSockets`
     property outside of the loop.
 - `timeout` - Integer containing the number of milliseconds to wait for a
-  request to respond before aborting the request.  Note that if the underlying
-  TCP connection cannot be established, the OS-wide TCP connection timeout will
-  overrule the `timeout` option ([the default in Linux is around 20 seconds](http://www.sekuda.com/overriding_the_default_linux_kernel_20_second_tcp_socket_connect_timeout)).
+server to send response headers (and start the response body) before aborting
+the request. Note that if the underlying TCP connection cannot be established,
+the OS-wide TCP connection timeout will overrule the `timeout` option ([the
+default in Linux can be anywhere from 20-120 seconds][linux-timeout]).
+
+[linux-timeout]: http://www.sekuda.com/overriding_the_default_linux_kernel_20_second_tcp_socket_connect_timeout
 
 ---
 
@@ -938,6 +941,35 @@ There are at least three ways to debug the operation of `request`:
 
 ---
 
+## Timeouts
+
+Most requests to external servers should have a timeout attached, in case the
+server is not responding in a timely manner. Without a timeout, your code may
+have a socket open/consume resources for minutes or more.
+
+There are two main types of timeouts: **connection timeouts** and **read
+timeouts**. A connect timeout occurs if the timeout is hit while your client is
+attempting to establish a connection to a remote machine (corresponding to the
+[connect() call][connect] on the socket). A read timeout occurs any time the
+server is too slow to send back a part of the response.
+
+These two situations have widely different implications for what went wrong
+with the request, so it's useful to be able to distinguish them. You can detect
+timeout errors by checking `err.code` for an 'ETIMEDOUT' value. Further, you
+can detect whether the timeout was a connection timeout by checking if the
+`err.connect` property is set to `true`.
+
+```js
+request.get('http://10.255.255.1', {timeout: 1500}, function(err) {
+    console.log(err.code === 'ETIMEDOUT');
+    // Set to `true` if the timeout was a connection timeout, `false` or
+    // `undefined` otherwise.
+    console.log(err.connect === true);
+    process.exit(0);
+});
+```
+
+[connect]: http://linux.die.net/man/2/connect
 
 ## Examples:
 

npm-debug.log

@@ -0,0 +1,36 @@
+0 info it worked if it ends with ok
+1 verbose cli [ 'node', '/opt/boxen/homebrew/bin/npm', 'run', 'test-cov' ]
+2 info using npm@2.13.3
+3 info using node@v0.12.7
+4 verbose node symlink /opt/boxen/homebrew/bin/node
+5 verbose run-script [ 'pretest-cov', 'test-cov', 'posttest-cov' ]
+6 info pretest-cov request@2.60.1
+7 info test-cov request@2.60.1
+8 verbose unsafe-perm in lifecycle true
+9 info request@2.60.1 Failed to exec test-cov script
+10 verbose stack Error: request@2.60.1 test-cov: `istanbul cover tape tests/test-*.js`
+10 verbose stack Exit status 1
+10 verbose stack     at EventEmitter.<anonymous> (/opt/boxen/homebrew/lib/node_modules/npm/lib/utils/lifecycle.js:214:16)
+10 verbose stack     at EventEmitter.emit (events.js:110:17)
+10 verbose stack     at ChildProcess.<anonymous> (/opt/boxen/homebrew/lib/node_modules/npm/lib/utils/spawn.js:24:14)
+10 verbose stack     at ChildProcess.emit (events.js:110:17)
+10 verbose stack     at maybeClose (child_process.js:1015:16)
+10 verbose stack     at Process.ChildProcess._handle.onexit (child_process.js:1087:5)
+11 verbose pkgid request@2.60.1
+12 verbose cwd /Users/burke/code/request
+13 error Darwin 14.4.0
+14 error argv "node" "/opt/boxen/homebrew/bin/npm" "run" "test-cov"
+15 error node v0.12.7
+16 error npm  v2.13.3
+17 error code ELIFECYCLE
+18 error request@2.60.1 test-cov: `istanbul cover tape tests/test-*.js`
+18 error Exit status 1
+19 error Failed at the request@2.60.1 test-cov script 'istanbul cover tape tests/test-*.js'.
+19 error This is most likely a problem with the request package,
+19 error not with npm itself.
+19 error Tell the author that this fails on your system:
+19 error     istanbul cover tape tests/test-*.js
+19 error You can get their info via:
+19 error     npm owner ls request
+19 error There is likely additional logging output above.
+20 verbose exit [ 1, true ]

request.js

@@ -804,21 +804,33 @@ Request.prototype.start = function () {
 
   if (self.timeout && !self.timeoutTimer) {
     var timeout = self.timeout < 0 ? 0 : self.timeout
+    // Set a timeout in memory - this block will throw if the server takes more
+    // than `timeout` to write the HTTP status and headers (corresponding to
+    // the on('response') event on the client). NB: this measures wall-clock
+    // time, not the time between bytes sent by the server.
     self.timeoutTimer = setTimeout(function () {
+      var connectTimeout = self.req.socket && self.req.socket.readable === false
       self.abort()
       var e = new Error('ETIMEDOUT')
       e.code = 'ETIMEDOUT'
+      e.connect = connectTimeout
       self.emit('error', e)
     }, timeout)
 
-    // Set additional timeout on socket - in case if remote
-    // server freeze after sending headers
     if (self.req.setTimeout) { // only works on node 0.6+
+      // Set an additional timeout on the socket, via the `setsockopt` syscall.
+      // This timeout sets the amount of time to wait *between* bytes sent
+      // from the server, and may or may not correspond to the wall-clock time
+      // elapsed from the start of the request.
+      //
+      // In particular, it's useful for erroring if the server fails to send
+      // data halfway through streaming a response.
       self.req.setTimeout(timeout, function () {
         if (self.req) {
           self.req.abort()
           var e = new Error('ESOCKETTIMEDOUT')
           e.code = 'ESOCKETTIMEDOUT'
+          e.connect = false
           self.emit('error', e)
         }
       })

tests/test-timeout.js

@@ -43,6 +43,19 @@ if (process.env.TRAVIS === 'true') {
     })
   })
 
+  tape('should set connect to false', function(t) {
+    var shouldTimeout = {
+      url: s.url + '/timeout',
+      timeout: 100
+    }
+
+    request(shouldTimeout, function(err, res, body) {
+      checkErrCode(t, err)
+      t.ok(err.connect === false, 'Read Timeout Error should set \'connect\' property to false')
+      t.end()
+    })
+  })
+
   tape('should timeout with events', function(t) {
     t.plan(3)
 
@@ -109,6 +122,22 @@ if (process.env.TRAVIS === 'true') {
     })
   })
 
+  tape('connect timeout', function(t) {
+    // We need a destination that will not immediately return a TCP Reset
+    // packet. StackOverflow suggests this host:
+    // https://stackoverflow.com/a/904609/329700
+    var tarpitHost = 'http://10.255.255.1'
+    var shouldConnectTimeout = {
+      url: tarpitHost + '/timeout',
+      timeout: 100
+    }
+    request(shouldConnectTimeout, function(err) {
+      checkErrCode(t, err)
+      t.ok(err.connect === true, 'Connect Timeout Error should set \'connect\' property to true')
+      t.end()
+    })
+  })
+
   tape('cleanup', function(t) {
     s.close(function() {
       t.end()