Skip to content

Commit ee4cfbb

Browse files
committed
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.
1 parent cf757e1 commit ee4cfbb

4 files changed

Lines changed: 79 additions & 5 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
node_modules
22
coverage
33
.idea
4+
npm-debug.log

‎README.md‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -784,9 +784,12 @@ The first argument can be either a `url` or an `options` object. The only requir
784784
with your pool options or create the pool object with the `maxSockets`
785785
property outside of the loop.
786786
- `timeout` - Integer containing the number of milliseconds to wait for a
787-
request to respond before aborting the request. Note that if the underlying
788-
TCP connection cannot be established, the OS-wide TCP connection timeout will
789-
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)).
787+
server to send response headers (and start the response body) before aborting
788+
the request. Note that if the underlying TCP connection cannot be established,
789+
the OS-wide TCP connection timeout will overrule the `timeout` option ([the
790+
default in Linux can be anywhere from 20-120 seconds][linux-timeout]).
791+
792+
[linux-timeout]: http://www.sekuda.com/overriding_the_default_linux_kernel_20_second_tcp_socket_connect_timeout
790793

791794
---
792795

@@ -938,6 +941,35 @@ There are at least three ways to debug the operation of `request`:
938941

939942
---
940943

944+
## Timeouts
945+
946+
Most requests to external servers should have a timeout attached, in case the
947+
server is not responding in a timely manner. Without a timeout, your code may
948+
have a socket open/consume resources for minutes or more.
949+
950+
There are two main types of timeouts: **connection timeouts** and **read
951+
timeouts**. A connect timeout occurs if the timeout is hit while your client is
952+
attempting to establish a connection to a remote machine (corresponding to the
953+
[connect() call][connect] on the socket). A read timeout occurs any time the
954+
server is too slow to send back a part of the response.
955+
956+
These two situations have widely different implications for what went wrong
957+
with the request, so it's useful to be able to distinguish them. You can detect
958+
timeout errors by checking `err.code` for an 'ETIMEDOUT' value. Further, you
959+
can detect whether the timeout was a connection timeout by checking if the
960+
`err.connect` property is set to `true`.
961+
962+
```js
963+
request.get('http://10.255.255.1', {timeout: 1500}, function(err) {
964+
console.log(err.code === 'ETIMEDOUT');
965+
// Set to `true` if the timeout was a connection timeout, `false` or
966+
// `undefined` otherwise.
967+
console.log(err.connect === true);
968+
process.exit(0);
969+
});
970+
```
971+
972+
[connect]: http://linux.die.net/man/2/connect
941973

942974
## Examples:
943975

‎request.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,21 +804,33 @@ Request.prototype.start = function () {
804804

805805
if (self.timeout && !self.timeoutTimer) {
806806
var timeout = self.timeout < 0 ? 0 : self.timeout
807+
// Set a timeout in memory - this block will throw if the server takes more
808+
// than `timeout` to write the HTTP status and headers (corresponding to
809+
// the on('response') event on the client). NB: this measures wall-clock
810+
// time, not the time between bytes sent by the server.
807811
self.timeoutTimer = setTimeout(function () {
812+
var connectTimeout = self.req.socket && self.req.socket.readable === false
808813
self.abort()
809814
var e = new Error('ETIMEDOUT')
810815
e.code = 'ETIMEDOUT'
816+
e.connect = connectTimeout
811817
self.emit('error', e)
812818
}, timeout)
813819

814-
// Set additional timeout on socket - in case if remote
815-
// server freeze after sending headers
816820
if (self.req.setTimeout) { // only works on node 0.6+
821+
// Set an additional timeout on the socket, via the `setsockopt` syscall.
822+
// This timeout sets the amount of time to wait *between* bytes sent
823+
// from the server, and may or may not correspond to the wall-clock time
824+
// elapsed from the start of the request.
825+
//
826+
// In particular, it's useful for erroring if the server fails to send
827+
// data halfway through streaming a response.
817828
self.req.setTimeout(timeout, function () {
818829
if (self.req) {
819830
self.req.abort()
820831
var e = new Error('ESOCKETTIMEDOUT')
821832
e.code = 'ESOCKETTIMEDOUT'
833+
e.connect = false
822834
self.emit('error', e)
823835
}
824836
})

‎tests/test-timeout.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,19 @@ if (process.env.TRAVIS === 'true') {
4343
})
4444
})
4545

46+
tape('should set connect to false', function(t) {
47+
var shouldTimeout = {
48+
url: s.url + '/timeout',
49+
timeout: 100
50+
}
51+
52+
request(shouldTimeout, function(err, res, body) {
53+
checkErrCode(t, err)
54+
t.ok(err.connect === false, 'Read Timeout Error should set \'connect\' property to false')
55+
t.end()
56+
})
57+
})
58+
4659
tape('should timeout with events', function(t) {
4760
t.plan(3)
4861

@@ -109,6 +122,22 @@ if (process.env.TRAVIS === 'true') {
109122
})
110123
})
111124

125+
tape('connect timeout', function(t) {
126+
// We need a destination that will not immediately return a TCP Reset
127+
// packet. StackOverflow suggests this host:
128+
// https://stackoverflow.com/a/904609/329700
129+
var tarpitHost = 'http://10.255.255.1'
130+
var shouldConnectTimeout = {
131+
url: tarpitHost + '/timeout',
132+
timeout: 100
133+
}
134+
request(shouldConnectTimeout, function(err) {
135+
checkErrCode(t, err)
136+
t.ok(err.connect === true, 'Connect Timeout Error should set \'connect\' property to true')
137+
t.end()
138+
})
139+
})
140+
112141
tape('cleanup', function(t) {
113142
s.close(function() {
114143
t.end()

0 commit comments

Comments
 (0)