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.

diff --git a/lib/http.js b/lib/http.js
index 7294414..d7c0083 100644 (file)
--- a/lib/http.js
+++ b/lib/http.js
@@ -976,12 +976,20 @@ function connectionListener(socket) {
   var self = this;
   var outgoing = [];
   var incoming = [];
+  var abortError = null;
 
   function abortIncoming() {
+    if (!abortError) {
+      abortError = new Error('http.ServerRequest was aborted by the client');
+      abortError.code = 'aborted';
+    }
+
     while (incoming.length) {
       var req = incoming.shift();
+
+      // @deprecated, should be removed in 0.5.x
       req.emit('aborted');
-      req.emit('close');
+      req.emit('close', abortError);
     }
     // abort socket._httpMessage ?
   }
@@ -992,6 +1000,8 @@ function connectionListener(socket) {
 
   socket.setTimeout(2 * 60 * 1000); // 2 minute timeout
   socket.addListener('timeout', function() {
+    abortError = new Error('http.ServerRequest timed out');
+    abortError.code = 'timeout';
     socket.destroy();
   });
 

diff --git a/test/simple/test-http-request-aborted.js b/test/simple/test-http-request-aborted.js
new file mode 100644 (file)
index 0000000..b80b0ed
--- /dev/null
+++ b/test/simple/test-http-request-aborted.js
@@ -0,0 +1,44 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var http = require('http');
+var net = require('net');
+
+var closeError;
+
+var server = http.Server(function(req, res) {
+  req.on('close', function(err) {
+    closeError = err;
+    server.close();
+  });
+});
+
+server.listen(common.PORT, function() {
+  var socket = net.createConnection(common.PORT);
+  socket.write('GET / HTTP/1.1\n\n');
+  socket.end();
+});
+
+process.on('exit', function() {
+  assert.equal(closeError.code, 'aborted');
+});

diff --git a/test/simple/test-http-set-timeout.js b/test/simple/test-http-set-timeout.js
index 641a425..fbc8f2c 100644 (file)
--- a/test/simple/test-http-set-timeout.js
+++ b/test/simple/test-http-set-timeout.js
@@ -24,11 +24,12 @@ var assert = require('assert');
 var http = require('http');
 
 var server = http.createServer(function(req, res) {
-  console.log('got request. setting 1 second timeout');
-  req.connection.setTimeout(500);
+  console.log('got request. setting 100ms second timeout');
+  req.connection.setTimeout(100);
+
+  req.on('close', function(err) {
+    assert.strictEqual(err.code, 'timeout');
 
-  req.connection.addListener('timeout', function() {
-    req.connection.destroy();
     common.debug('TIMEOUT');
     server.close();
   });

diff --git a/test/simple/test-https-request-aborted.js b/test/simple/test-https-request-aborted.js
new file mode 100644 (file)
index 0000000..d3d7ac4
--- /dev/null
+++ b/test/simple/test-https-request-aborted.js
@@ -0,0 +1,53 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var https = require('https');
+var fs = require('fs');
+
+var closeError;
+
+var options = {
+  key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
+  cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
+};
+
+var server = https.Server(options, function(req, res) {
+  res.writeHead(200);
+  res.write('Hi');
+
+  req.on('close', function(err) {
+    closeError = err;
+    server.close();
+  });
+});
+
+server.listen(common.PORT, function() {
+  https.get({port: common.PORT, path: '/'}, function(res) {
+    res.socket.end();
+  })
+});
+
+process.on('exit', function() {
+  console.log(closeError);
+  assert.equal(closeError.code, 'aborted');
+});

diff --git a/test/simple/test-https-request-timeout.js b/test/simple/test-https-request-timeout.js
new file mode 100644 (file)
index 0000000..14335ed
--- /dev/null
+++ b/test/simple/test-https-request-timeout.js
@@ -0,0 +1,53 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var https = require('https');
+var fs = require('fs');
+
+var closeError;
+
+var options = {
+  key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
+  cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
+};
+
+var server = https.Server(options, function(req, res) {
+  req.connection.setTimeout(100);
+  res.writeHead(200);
+  res.write('Hi');
+
+  req.on('close', function(err) {
+    closeError = err;
+    server.close();
+  });
+});
+
+server.listen(common.PORT, function() {
+  https.get({port: common.PORT, path: '/'}, function(res) {
+
+  })
+});
+
+process.on('exit', function() {
+  assert.equal(closeError.code, 'timeout');
+});