Skip to content

Commit

Permalink
Merge pull request #12 from lpinca/fix/socket-timeout
Browse files Browse the repository at this point in the history
Use request.setTimeout() instead of socket.setTimeout()
  • Loading branch information
floatdrop authored Jan 16, 2017
2 parents 41347e6 + 56933a3 commit e216d30
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 13 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
language: node_js
node_js:
- '0.10'
- '0.12'
- '4'
- '5'
- '6'
6 changes: 3 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module.exports = function (req, time) {
return req;
}

var delays = isNaN(time) ? time : {connect: time};
var delays = isNaN(time) ? time : {socket: time, connect: time};
var host = req._headers ? (' to ' + req._headers.host) : '';

if (delays.connect !== undefined) {
Expand All @@ -22,7 +22,7 @@ module.exports = function (req, time) {
req.on('socket', function assign(socket) {
// Socket may come from Agent pool and may be already connected.
if (!(socket.connecting || socket._connecting)) {
connect.call(socket);
connect();
return;
}

Expand All @@ -42,7 +42,7 @@ module.exports = function (req, time) {
if (delays.socket !== undefined) {
// Abort the request if there is no activity on the socket for more
// than `delays.socket` milliseconds.
this.setTimeout(delays.socket, function socketTimeoutHandler() {
req.setTimeout(delays.socket, function socketTimeoutHandler() {
req.abort();
var e = new Error('Socket timed out on request' + host);
e.code = 'ESOCKETTIMEDOUT';
Expand Down
4 changes: 2 additions & 2 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ The request to watch on.
*Required*
Type: `number` or `object`

Time in milliseconds to wait for `connect` event on socket.
Time in milliseconds to wait for `connect` event on socket and also time to wait on inactive socket.

Or you can pass Object with following fields:

- `connect` - time to wait for connection
- `socket` - time to wait for activity on socket (this can break sockets from keep-alive pool)
- `socket` - time to wait for activity on socket

## License

Expand Down
52 changes: 46 additions & 6 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ it('should emit ETIMEDOUT when connection timeout expires', function (done) {
}
});

timeout(req, {connect: 200});
timeout(req, 200);
});

describe('when connection is established', function () {
Expand All @@ -57,7 +57,7 @@ describe('when connection is established', function () {
}
});

timeout(req, {socket: 200});
timeout(req, 200);
});

it('should emit ESOCKETTIMEDOUT (only first chunk of body)', function (done) {
Expand Down Expand Up @@ -132,7 +132,7 @@ describe('when connection is established', function () {

// Different requests may reuse one socket if keep-alive is enabled
it('should not add event handlers twice for the same socket', function (done) {
server.once('request', function (req, res) {
server.on('request', function (req, res) {
res.writeHead(200);
res.end('data');
});
Expand All @@ -154,16 +154,17 @@ describe('when connection is established', function () {
var req2 = http.get(reqOpts, function (resp) {
resp.resume();
keepAliveAgent.destroy();
server.removeAllListeners('request');
done();
});
timeout(req2, 100);

req2.on('socket', function (sock) {
assert.equal(sock, socket);
assert.equal(sock.listeners('connect').length, 0);
done();
});
});
timeout(req1, {socket: 100, connect: 100});
timeout(req1, 100);

req1.on('socket', function (sock) {
socket = sock;
Expand All @@ -188,7 +189,46 @@ describe('when connection is established', function () {
}
});

timeout(req, {socket: 200, connect: 200});
timeout(req, 200);
});
});

it('should clear socket timeout for keep-alive sockets', function (done) {
server.once('request', function (req, res) {
res.writeHead(200);
res.end('data');
});

var socket = null;
var agent = new http.Agent({
keepAlive: true,
maxSockets: 1
});

var options = {
hostname: '0.0.0.0',
agent: agent,
port: 8081
};

var req = http.get(options, function (res) {
assert.equal(socket._idleTimeout, 100);
res.resume();
res.on('end', function () {
assert.equal(socket.destroyed, false);
assert.equal(socket._idleTimeout, -1);
agent.destroy();
done();
});
});

timeout(req, 100);

req.on('socket', function (sock) {
sock.once('connect', function () {
assert.equal(sock._idleTimeout, 100);
});
socket = sock;
});
});
});

0 comments on commit e216d30

Please sign in to comment.