http: emit 'error' on aborted server request

Server requests aka. IncomingMessage emits 'aborted' instead of 'error' which causes confusion when the object is used as a regular stream, i.e. if functions working on streams are passed a server request object they might not work properly unless they take this into account. Refs: nodejs/web-server-frameworks#41
nxtedition · May 1, 2020 · 9515204 · 9515204
1 parent e10e292
commit 9515204
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 17 deletions.
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -56,12 +56,16 @@ const {
   getOrSetAsyncId
 } = require('internal/async_hooks');
 const { IncomingMessage } = require('_http_incoming');
+const {
+  connResetException,
+  codes
+} = require('internal/errors');
 const {
   ERR_HTTP_HEADERS_SENT,
   ERR_HTTP_INVALID_STATUS_CODE,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_CHAR
-} = require('internal/errors').codes;
+} = codes;
 const { validateInteger } = require('internal/validators');
 const Buffer = require('buffer').Buffer;
 const {
@@ -533,8 +537,12 @@ function socketOnClose(socket, state) {
 function abortIncoming(incoming) {
   while (incoming.length) {
     const req = incoming.shift();
+    // TODO(ronag): req.destroy(err)
     req.aborted = true;
     req.emit('aborted');
+    if (req.listenerCount('error') > 0) {
+      req.emit('error', connResetException('aborted'));
+    }
     req.emit('close');
   }
   // Abort socket._httpMessage ?

diff --git a/test/parallel/test-http-aborted.js b/test/parallel/test-http-aborted.js
@@ -4,23 +4,52 @@ const common = require('../common');
 const http = require('http');
 const assert = require('assert');
 
-const server = http.createServer(common.mustCall(function(req, res) {
-  req.on('aborted', common.mustCall(function() {
-    assert.strictEqual(this.aborted, true);
-    server.close();
+{
+  const server = http.createServer(common.mustCall(function(req, res) {
+    req.on('aborted', common.mustCall(function() {
+      assert.strictEqual(this.aborted, true);
+    }));
+    req.on('error', common.mustCall(function(err) {
+      assert.strictEqual(err.code, 'ECONNRESET');
+      server.close();
+    }));
+    assert.strictEqual(req.aborted, false);
+    res.write('hello');
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    const req = http.get({
+      port: server.address().port,
+      headers: { connection: 'keep-alive' }
+    }, common.mustCall((res) => {
+      res.on('aborted', common.mustCall(() => {
+        assert.strictEqual(res.aborted, true);
+      }));
+      req.abort();
+    }));
+  }));
+}
+
+{
+  // Don't crash if no 'error' handler on server request.
+
+  const server = http.createServer(common.mustCall(function(req, res) {
+    req.on('aborted', common.mustCall(function() {
+      assert.strictEqual(this.aborted, true);
+    }));
+    assert.strictEqual(req.aborted, false);
+    res.write('hello');
   }));
-  assert.strictEqual(req.aborted, false);
-  res.write('hello');
-}));
 
-server.listen(0, common.mustCall(() => {
-  const req = http.get({
-    port: server.address().port,
-    headers: { connection: 'keep-alive' }
-  }, common.mustCall((res) => {
-    res.on('aborted', common.mustCall(() => {
-      assert.strictEqual(res.aborted, true);
+  server.listen(0, common.mustCall(() => {
+    const req = http.get({
+      port: server.address().port,
+      headers: { connection: 'keep-alive' }
+    }, common.mustCall((res) => {
+      res.on('aborted', common.mustCall(() => {
+        assert.strictEqual(res.aborted, true);
+      }));
+      req.abort();
     }));
-    req.abort();
   }));
-}));
+}