Skip to content

Commit

Permalink
http,net,tls: return this from setTimeout methods
Browse files Browse the repository at this point in the history
Modifies the setTimeout methods for the following prototypes:

- http.ClientRequest
- http.IncomingMessage
- http.OutgoingMessage
- http.Server
- https.Server
- net.Socket
- tls.TLSSocket

Previously, the above functions returned undefined. They now return
`this`. This is useful for chaining function calls.

PR-URL: #1699
Reviewed-By: Roman Reiss <[email protected]>
  • Loading branch information
evanlucas authored and silverwind committed May 16, 2015
1 parent c7fb91d commit d4726cd
Show file tree
Hide file tree
Showing 16 changed files with 48 additions and 18 deletions.
8 changes: 8 additions & 0 deletions doc/api/http.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ destroyed automatically if they time out. However, if you assign a
callback to the Server's `'timeout'` event, then you are responsible
for handling socket timeouts.

Returns `server`.

### server.timeout

* {Number} Default = 120000 (2 minutes)
Expand Down Expand Up @@ -318,6 +320,8 @@ assign a handler on the request, the response, or the server's
`'timeout'` events, then it is your responsibility to handle timed out
sockets.

Returns `response`.

### response.statusCode

When using implicit headers (not calling [response.writeHead()][] explicitly),
Expand Down Expand Up @@ -914,6 +918,8 @@ Aborts a request. (New since v0.3.8.)
Once a socket is assigned to this request and is connected
[socket.setTimeout()][] will be called.

Returns `request`.

### request.setNoDelay([noDelay])

Once a socket is assigned to this request and is connected
Expand Down Expand Up @@ -1003,6 +1009,8 @@ received. Only populated at the 'end' event.

Calls `message.connection.setTimeout(msecs, callback)`.

Returns `message`.

### message.method

**Only valid for request obtained from [http.Server][].**
Expand Down
4 changes: 3 additions & 1 deletion doc/api/net.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ For TCP sockets, `options` argument should be an object which specifies:
- `localPort`: Local port to bind to for network connections.

- `family` : Version of IP stack. Defaults to `4`.

- `lookup` : Custom lookup function. Defaults to `dns.lookup`.

For local domain sockets, `options` argument should be an object which
Expand Down Expand Up @@ -451,6 +451,8 @@ If `timeout` is 0, then the existing idle timeout is disabled.
The optional `callback` parameter will be added as a one time listener for the
`'timeout'` event.

Returns `socket`.

### socket.setNoDelay([noDelay])

Disables the Nagle algorithm. By default TCP connections use the Nagle
Expand Down
6 changes: 4 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ ClientRequest.prototype.setTimeout = function(msecs, callback) {
this.socket.setTimeout(0, this.timeoutCb);
this.timeoutCb = emitTimeout;
this.socket.setTimeout(msecs, emitTimeout);
return;
return this;
}

// Set timeoutCb so that it'll get cleaned up on request end
Expand All @@ -548,12 +548,14 @@ ClientRequest.prototype.setTimeout = function(msecs, callback) {
this.socket.once('connect', function() {
sock.setTimeout(msecs, emitTimeout);
});
return;
return this;
}

this.once('socket', function(sock) {
sock.setTimeout(msecs, emitTimeout);
});

return this;
};

ClientRequest.prototype.setNoDelay = function() {
Expand Down
1 change: 1 addition & 0 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ IncomingMessage.prototype.setTimeout = function(msecs, callback) {
if (callback)
this.on('timeout', callback);
this.socket.setTimeout(msecs);
return this;
};


Expand Down
1 change: 1 addition & 0 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ OutgoingMessage.prototype.setTimeout = function(msecs, callback) {
} else {
this.socket.setTimeout(msecs);
}
return this;
};


Expand Down
1 change: 1 addition & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ Server.prototype.setTimeout = function(msecs, callback) {
this.timeout = msecs;
if (callback)
this.on('timeout', callback);
return this;
};


Expand Down
1 change: 1 addition & 0 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ Socket.prototype.setTimeout = function(msecs, callback) {
this.once('timeout', callback);
}
}
return this;
};


Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http-client-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ server.listen(options.port, options.host, function() {
function destroy() {
req.destroy();
}
req.setTimeout(1, destroy);
var s = req.setTimeout(1, destroy);
assert.ok(s instanceof http.ClientRequest);
req.on('error', destroy);
req.end();
});
18 changes: 12 additions & 6 deletions test/parallel/test-http-set-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ test(function serverTimeout(cb) {
// just do nothing, we should get a timeout event.
});
server.listen(common.PORT);
server.setTimeout(50, function(socket) {
var s = server.setTimeout(50, function(socket) {
caughtTimeout = true;
socket.destroy();
server.close();
cb();
});
assert.ok(s instanceof http.Server);
http.get({ port: common.PORT }).on('error', function() {});
});

Expand All @@ -45,12 +46,13 @@ test(function serverRequestTimeout(cb) {
});
var server = http.createServer(function(req, res) {
// just do nothing, we should get a timeout event.
req.setTimeout(50, function() {
var s = req.setTimeout(50, function() {
caughtTimeout = true;
req.socket.destroy();
server.close();
cb();
});
assert.ok(s instanceof http.IncomingMessage);
});
server.listen(common.PORT);
var req = http.request({ port: common.PORT, method: 'POST' });
Expand All @@ -66,12 +68,13 @@ test(function serverResponseTimeout(cb) {
});
var server = http.createServer(function(req, res) {
// just do nothing, we should get a timeout event.
res.setTimeout(50, function() {
var s = res.setTimeout(50, function() {
caughtTimeout = true;
res.socket.destroy();
server.close();
cb();
});
assert.ok(s instanceof http.OutgoingMessage);
});
server.listen(common.PORT);
http.get({ port: common.PORT }).on('error', function() {});
Expand All @@ -86,9 +89,10 @@ test(function serverRequestNotTimeoutAfterEnd(cb) {
});
var server = http.createServer(function(req, res) {
// just do nothing, we should get a timeout event.
req.setTimeout(50, function(socket) {
var s = req.setTimeout(50, function(socket) {
caughtTimeoutOnRequest = true;
});
assert.ok(s instanceof http.IncomingMessage);
res.on('timeout', function(socket) {
caughtTimeoutOnResponse = true;
});
Expand All @@ -108,9 +112,10 @@ test(function serverResponseTimeoutWithPipeline(cb) {
assert.equal(caughtTimeout, '/2');
});
var server = http.createServer(function(req, res) {
res.setTimeout(50, function() {
var s = res.setTimeout(50, function() {
caughtTimeout += req.url;
});
assert.ok(s instanceof http.OutgoingMessage);
if (req.url === '/1') res.end();
});
server.on('timeout', function(socket) {
Expand Down Expand Up @@ -144,12 +149,13 @@ test(function idleTimeout(cb) {
});
res.end();
});
server.setTimeout(50, function(socket) {
var s = server.setTimeout(50, function(socket) {
caughtTimeoutOnServer = true;
socket.destroy();
server.close();
cb();
});

This comment has been minimized.

Copy link
@Qh0stM4N

Qh0stM4N Jun 7, 2016

//where until variable s ?

assert.ok(s instanceof http.Server);
server.listen(common.PORT);
var c = net.connect({ port: common.PORT, allowHalfOpen: true }, function() {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-http-set-timeout.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
var common = require('../common');
var assert = require('assert');
var http = require('http');
var net = require('net');

var server = http.createServer(function(req, res) {
console.log('got request. setting 1 second timeout');
req.connection.setTimeout(500);

var s = req.connection.setTimeout(500);
assert.ok(s instanceof net.Socket);
req.connection.on('timeout', function() {
req.connection.destroy();
common.debug('TIMEOUT');
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-https-set-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,13 @@ test(function serverTimeout(cb) {
// just do nothing, we should get a timeout event.
});
server.listen(common.PORT);
server.setTimeout(50, function(socket) {
var s = server.setTimeout(50, function(socket) {
caughtTimeout = true;
socket.destroy();
server.close();
cb();
});
assert.ok(s instanceof https.Server);
https.get({
port: common.PORT,
rejectUnauthorized: false
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-https-timeout-server-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ var options = {
var server = https.createServer(options, assert.fail);

server.on('secureConnection', function(cleartext) {
cleartext.setTimeout(50, function() {
var s = cleartext.setTimeout(50, function() {
cleartext.destroy();
server.close();
});
assert.ok(s instanceof tls.TLSSocket);
});

server.listen(common.PORT, function() {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-net-settimeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ var left = killers.length;
killers.forEach(function(killer) {
var socket = net.createConnection(common.PORT, 'localhost');

socket.setTimeout(T, function() {
var s = socket.setTimeout(T, function() {
socket.destroy();
if (--left === 0) server.close();
assert.ok(killer !== 0);
clearTimeout(timeout);
});
assert.ok(s instanceof net.Socket);

socket.setTimeout(killer);

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-request-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ var options = {
};

var server = tls.Server(options, function(socket) {
socket.setTimeout(100);
var s = socket.setTimeout(100);
assert.ok(s instanceof tls.TLSSocket);

socket.on('timeout', function(err) {
hadTimeout = true;
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-timeout-server-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ var options = {
};

var server = tls.createServer(options, function(cleartext) {
cleartext.setTimeout(50, function() {
var s = cleartext.setTimeout(50, function() {
cleartext.destroy();
server.close();
});
assert.ok(s instanceof tls.TLSSocket);
});

server.listen(common.PORT, function() {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-wrap-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ var server = tls.createServer(options, function(c) {

server.listen(common.PORT, function() {
var socket = net.connect(common.PORT, function() {
socket.setTimeout(common.platformTimeout(240), function() {
var s = socket.setTimeout(common.platformTimeout(240), function() {
throw new Error('timeout');
});
assert.ok(s instanceof net.Socket);

var tsocket = tls.connect({
socket: socket,
Expand Down

4 comments on commit d4726cd

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semver: minor

@silverwind
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR was tagged as minor, did I miss anything?

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait did we give up on including that in the commit message? Sorry, forgot. Carry on. :)

@silverwind
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't heard of that practice yet. I checked the last semver commits, and they didn't include such a tag. If such a tag is prefered, please add it to COLLABORATOR_GUIDE.md ;)

Please sign in to comment.