From 0e106ecd039297b405160ff71f331330900a3dc8 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Mon, 3 Aug 2015 05:11:21 -0700 Subject: [PATCH] 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 | 38 +++++++++++++++++++++++++++++++++++--- request.js | 17 ++++++++++++++--- tests/test-timeout.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b72276798..a477ad551 100644 --- a/README.md +++ b/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-150 seconds 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: diff --git a/request.js b/request.js index a4af56a97..053e98270 100644 --- a/request.js +++ b/request.js @@ -804,21 +804,32 @@ 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) } }) diff --git a/tests/test-timeout.js b/tests/test-timeout.js index 9037c8b88..ac45867a7 100644 --- a/tests/test-timeout.js +++ b/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()