http2: emit error on canceled streams when aborted event is not emitted

lib/internal/http2/core.js

@@ -2409,8 +2409,20 @@ class Http2Stream extends Duplex {
       validateFunction(callback, 'callback');
     }
 
-    if (this.closed)
+    if (this.closed) {
+      // A client stream may already have been marked as closed with
+      // NGHTTP2_NO_ERROR by the time the session or underlying socket is
+      // canceled. Preserve the cancelation code so _destroy() can still emit
+      // the expected stream error when the aborted event was not emitted.
+      if (code === NGHTTP2_CANCEL &&
+          this[kSession] !== undefined &&
+          this[kSession][kType] === NGHTTP2_SESSION_CLIENT &&
+          this.rstCode === NGHTTP2_NO_ERROR &&
+          !this.aborted) {
+        this[kState].rstCode = code;
+      }
       return;
+    }
 
     if (callback !== undefined)
       this.once('close', callback);
@@ -2468,11 +2480,17 @@ class Http2Stream extends Duplex {
     sessionState.writeQueueSize -= state.writeQueueSize;
     state.writeQueueSize = 0;
 
-    // RST code 8 not emitted as an error as its used by clients to signify
-    // abort and is already covered by aborted event, also allows more
-    // seamless compatibility with http1
-    if (err == null && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL)
+    // RST code 8 is commonly used by clients to signify abort and is already
+    // covered by the aborted event, which also keeps better compatibility with
+    // http1.
+    // However, if the aborted event was not emitted (e.g. because the
+    // writable side was already ended), client streams must still report the
+    // cancelation as an error.
+    if (err == null && code !== NGHTTP2_NO_ERROR &&
+        (code !== NGHTTP2_CANCEL ||
+         session[kType] === NGHTTP2_SESSION_CLIENT && !this.aborted)) {
       err = new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code);
+    }
 
     this[kSession] = undefined;
     this[kHandle] = undefined;

test/parallel/test-http2-client-cancel-stream-after-end.js

@@ -0,0 +1,43 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const h2 = require('http2');
+
+// Regression test for https://github.com/nodejs/node/issues/56627
+// When a client stream's writable side is already ended (e.g. GET request)
+// and the server destroys the session, the client stream should emit an
+// error event with RST code NGHTTP2_CANCEL, since the 'aborted' event
+// cannot be emitted when the writable side is already ended.
+
+{
+  const server = h2.createServer();
+  server.on('stream', common.mustCall((stream) => {
+    stream.session.destroy();
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    const client = h2.connect(`http://localhost:${server.address().port}`);
+
+    client.on('close', common.mustCall(() => {
+      server.close();
+    }));
+
+    const req = client.request();
+
+    req.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR');
+      assert.match(err.message, /NGHTTP2_CANCEL/);
+    }));
+
+    req.on('aborted', common.mustNotCall());
+
+    req.on('close', common.mustCall(() => {
+      assert.strictEqual(req.rstCode, h2.constants.NGHTTP2_CANCEL);
+    }));
+
+    req.resume();
+  }));
+}

test/parallel/test-http2-client-destroy.js

@@ -118,7 +118,10 @@
       client.destroy();
     });
 
-    client.request();
+    const req = client.request();
+    req.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR');
+    }));
   }));
 }
 

test/parallel/test-http2-client-jsstream-destroy.js

@@ -45,6 +45,7 @@ server.listen(0, common.mustCall(function() {
       createConnection: () => proxy
     });
     const req = client.request();
+    req.on('error', common.mustCall());
 
     server.on('request', () => {
       socket.destroy();

test/parallel/test-http2-client-socket-destroy.js

@@ -26,14 +26,15 @@ server.on('stream', common.mustCall((stream) => {
 server.listen(0, common.mustCall(function() {
   const client = h2.connect(`http://localhost:${this.address().port}`);
   const req = client.request();
+  req.on('error', common.mustCall());
 
   req.on('response', common.mustCall(() => {
     // Send a premature socket close
     client[kSocket].destroy();
   }));
 
   req.resume();
-  req.on('end', common.mustCall());
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => server.close()));
 
   // On the client, the close event must call

test/parallel/test-http2-respond-with-file-connection-abort.js

@@ -13,7 +13,12 @@ const {
 
 const server = http2.createServer();
 server.on('stream', common.mustCall((stream) => {
-  stream.on('error', common.mustCallAtLeast((err) => assert.strictEqual(err.code, 'ECONNRESET'), 0));
+  stream.on('error', common.mustCallAtLeast((err) => {
+    assert.ok(
+      err.code === 'ECONNRESET' || err.code === 'ERR_HTTP2_STREAM_ERROR',
+      `Unexpected error code: ${err.code}`
+    );
+  }, 0));
   stream.respondWithFile(process.execPath, {
     [HTTP2_HEADER_CONTENT_TYPE]: 'application/octet-stream'
   });
@@ -22,11 +27,11 @@ server.on('stream', common.mustCall((stream) => {
 server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
+  req.on('error', common.mustCall());
 
   req.on('response', common.mustCall());
   req.once('data', common.mustCall(() => {
     net.Socket.prototype.destroy.call(client.socket);
     server.close();
   }));
-  req.end();
 }));

test/parallel/test-http2-server-shutdown-options-errors.js

@@ -58,6 +58,7 @@
   common.mustCall(() => {
     const client = http2.connect(`http://localhost:${server.address().port}`);
     const req = client.request();
+    req.on('error', common.mustCall());
     req.resume();
     req.on('close', common.mustCall(() => {
       client.close();

test/parallel/test-http2-server-stream-session-destroy.js

@@ -1,3 +1,3 @@
 'use strict';
 
 const common = require('../common');
@@ -52,7 +52,8 @@
 server.listen(0, common.mustCall(() => {
   const client = h2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
+  req.on('error', common.mustCall());
   req.resume();
-  req.on('end', common.mustCall());
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => server.close(common.mustCall())));
 }));

test/parallel/test-http2-zero-length-header.js

@@ -22,5 +22,6 @@
 }));
 server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}/`);
-  client.request({ ':path': '/', '': 'foo', 'bar': '' }).end();
+  const req = client.request({ ':path': '/', '': 'foo', 'bar': '' });
+  req.on('error', common.mustCall());
 }));