Skip to content

Commit

Permalink
Ensure socket 'connect' listener is always removed
Browse files Browse the repository at this point in the history
Fixes: #2438
  • Loading branch information
mscdex committed Oct 26, 2016
1 parent 53b7831 commit bc15d2b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
7 changes: 5 additions & 2 deletions request.js
Expand Up @@ -767,7 +767,8 @@ Request.prototype.start = function () {
})
self.req.on('socket', function(socket) {
if (timeout !== undefined) {
socket.once('connect', function() {
var onReqSockConnect = function() {
socket.removeListener('connect', onReqSockConnect)
clearTimeout(self.timeoutTimer)
self.timeoutTimer = null
// Set an additional timeout on the socket, via the `setsockopt` syscall.
Expand All @@ -785,13 +786,15 @@ Request.prototype.start = function () {
self.emit('error', e)
}
})
})
}
socket.on('connect', onReqSockConnect)

// 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 () {
socket.removeListener('connect', onReqSockConnect)
self.abort()
var e = new Error('ETIMEDOUT')
e.code = 'ETIMEDOUT'
Expand Down
24 changes: 24 additions & 0 deletions tests/test-timeout.js
Expand Up @@ -75,10 +75,22 @@ tape('should not timeout', function(t) {
timeout: 1200
}

var socket
request(shouldntTimeout, function(err, res, body) {
t.equal(err, null)
t.equal(body, 'waited')

var connectListeners = socket.listeners('connect')
for (var i = 0; i < connectListeners.length; ++i) {
var fn = connectListeners[i]
if (typeof fn === 'function' && fn.name === 'onReqSockConnect') {
t.ok(false, 'Connect listener should not exist')
}
}

t.end()
}).on('socket', function(socket_) {
socket = socket_
})
})

Expand Down Expand Up @@ -131,10 +143,22 @@ tape('connect timeout', function(t) {
url: tarpitHost + '/timeout',
timeout: 100
}
var socket
request(shouldConnectTimeout, function(err) {
checkErrCode(t, err)
t.ok(err.connect === true, 'Connect Timeout Error should set \'connect\' property to true')

var connectListeners = socket.listeners('connect')
for (var i = 0; i < connectListeners.length; ++i) {
var fn = connectListeners[i]
if (typeof fn === 'function' && fn.name === 'onReqSockConnect') {
t.ok(false, 'Connect listener should not exist')
}
}

t.end()
}).on('socket', function(socket_) {
socket = socket_
})
})

Expand Down

0 comments on commit bc15d2b

Please sign in to comment.