Skip to content

Commit e7ac6d8

Browse files
felixgery
authored andcommitted
Error argument for http.ServerRequest 'close'
Problem: It was not possible to detect the reason for a premature connection termination in http requests. This patch provides a new `err` argument to the 'close' event which can be inspected to differentiate between a timeout and a client actively terminating the connection. Also contains tests for this new behavior for http and https.
1 parent 1fde5f5 commit e7ac6d8

5 files changed

Lines changed: 166 additions & 5 deletions

File tree

lib/http.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,12 +976,20 @@ function connectionListener(socket) {
976976
var self = this;
977977
var outgoing = [];
978978
var incoming = [];
979+
var abortError = null;
979980

980981
function abortIncoming() {
982+
if (!abortError) {
983+
abortError = new Error('http.ServerRequest was aborted by the client');
984+
abortError.code = 'aborted';
985+
}
986+
981987
while (incoming.length) {
982988
var req = incoming.shift();
989+
990+
// @deprecated, should be removed in 0.5.x
983991
req.emit('aborted');
984-
req.emit('close');
992+
req.emit('close', abortError);
985993
}
986994
// abort socket._httpMessage ?
987995
}
@@ -992,6 +1000,8 @@ function connectionListener(socket) {
9921000

9931001
socket.setTimeout(2 * 60 * 1000); // 2 minute timeout
9941002
socket.addListener('timeout', function() {
1003+
abortError = new Error('http.ServerRequest timed out');
1004+
abortError.code = 'timeout';
9951005
socket.destroy();
9961006
});
9971007

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
var http = require('http');
25+
var net = require('net');
26+
27+
var closeError;
28+
29+
var server = http.Server(function(req, res) {
30+
req.on('close', function(err) {
31+
closeError = err;
32+
server.close();
33+
});
34+
});
35+
36+
server.listen(common.PORT, function() {
37+
var socket = net.createConnection(common.PORT);
38+
socket.write('GET / HTTP/1.1\n\n');
39+
socket.end();
40+
});
41+
42+
process.on('exit', function() {
43+
assert.equal(closeError.code, 'aborted');
44+
});

test/simple/test-http-set-timeout.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@ var assert = require('assert');
2424
var http = require('http');
2525

2626
var server = http.createServer(function(req, res) {
27-
console.log('got request. setting 1 second timeout');
28-
req.connection.setTimeout(500);
27+
console.log('got request. setting 100ms second timeout');
28+
req.connection.setTimeout(100);
29+
30+
req.on('close', function(err) {
31+
assert.strictEqual(err.code, 'timeout');
2932

30-
req.connection.addListener('timeout', function() {
31-
req.connection.destroy();
3233
common.debug('TIMEOUT');
3334
server.close();
3435
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
var https = require('https');
25+
var fs = require('fs');
26+
27+
var closeError;
28+
29+
var options = {
30+
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
31+
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
32+
};
33+
34+
var server = https.Server(options, function(req, res) {
35+
res.writeHead(200);
36+
res.write('Hi');
37+
38+
req.on('close', function(err) {
39+
closeError = err;
40+
server.close();
41+
});
42+
});
43+
44+
server.listen(common.PORT, function() {
45+
https.get({port: common.PORT, path: '/'}, function(res) {
46+
res.socket.end();
47+
})
48+
});
49+
50+
process.on('exit', function() {
51+
console.log(closeError);
52+
assert.equal(closeError.code, 'aborted');
53+
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
var https = require('https');
25+
var fs = require('fs');
26+
27+
var closeError;
28+
29+
var options = {
30+
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
31+
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
32+
};
33+
34+
var server = https.Server(options, function(req, res) {
35+
req.connection.setTimeout(100);
36+
res.writeHead(200);
37+
res.write('Hi');
38+
39+
req.on('close', function(err) {
40+
closeError = err;
41+
server.close();
42+
});
43+
});
44+
45+
server.listen(common.PORT, function() {
46+
https.get({port: common.PORT, path: '/'}, function(res) {
47+
48+
})
49+
});
50+
51+
process.on('exit', function() {
52+
assert.equal(closeError.code, 'timeout');
53+
});

0 commit comments

Comments
 (0)