http2: fix destroy() to send and handle remote NGHTTP2_CANCEL

lib/internal/http2/core.js

@@ -2182,9 +2182,10 @@ class Http2Stream extends Duplex {
     );
     const hasHandle = handle !== undefined;
 
-    if (!this.closed)
-      closeStream(this, code, hasHandle ? kForceRstStream : kNoRstStream);
-    this.push(null);
+    if (!this.closed) {
+      const rstStreamStatus = hasHandle ? kForceRstStream : kNoRstStream;
+      closeStream(this, code || NGHTTP2_CANCEL, rstStreamStatus);
+    }
 
     if (hasHandle) {
       handle.destroy();
@@ -2200,9 +2201,14 @@ class Http2Stream extends Duplex {
     // 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)
+    if (err == null &&
+        code !== NGHTTP2_NO_ERROR &&
+        (code !== NGHTTP2_CANCEL || !this.aborted))
       err = new ERR_HTTP2_STREAM_ERROR(nameForErrorCode[code] || code);
 
+    if (!err)
+      this.push(null);
+
     this[kSession] = undefined;
     this[kHandle] = undefined;
 

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

@@ -116,7 +116,7 @@ const Countdown = require('../common/countdown');
       client.destroy();
     });
 
-    client.request();
+    client.request().on('error', common.expectsError());
   }));
 }
 
@@ -161,7 +161,7 @@ const Countdown = require('../common/countdown');
 
     client.close();
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => server.close()));
   }));
 }

test/parallel/test-http2-client-onconnect-errors.js

@@ -102,7 +102,7 @@ function runTest(test) {
     });
   }
 
-  req.on('end', common.mustCall());
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => {
     client.destroy();
 

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

@@ -32,7 +32,12 @@ server.listen(0, common.mustCall(function() {
   }));
 
   req.resume();
-  req.on('end', common.mustCall());
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => server.close()));
 
   // On the client, the close event must call

test/parallel/test-http2-compat-serverresponse-destroy.js

@@ -44,10 +44,15 @@ server.listen(0, common.mustCall(() => {
   {
     const req = client.request();
     req.on('response', common.mustNotCall());
-    req.on('error', common.mustNotCall());
-    req.on('end', common.mustCall());
+    req.on('error', common.expectsError({
+      code: 'ERR_HTTP2_STREAM_ERROR',
+      name: 'Error',
+      message: 'Stream closed with error code NGHTTP2_CANCEL'
+    }));
     req.on('close', common.mustCall(() => countdown.dec()));
+
     req.resume();
+    req.on('end', common.mustNotCall());
   }
 
   {
@@ -62,7 +67,7 @@ server.listen(0, common.mustCall(() => {
     req.on('close', common.mustCall(() => countdown.dec()));
 
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
   }
 
   {
@@ -77,6 +82,6 @@ server.listen(0, common.mustCall(() => {
     req.on('close', common.mustCall(() => countdown.dec()));
 
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
   }
 }));

test/parallel/test-http2-compat-serverresponse-headers-after-destroy.js

@@ -38,7 +38,13 @@ server.listen(0, common.mustCall(function() {
       ':authority': `localhost:${port}`
     };
     const request = client.request(headers);
-    request.on('end', common.mustCall(function() {
+    request.on('error', common.expectsError({
+      code: 'ERR_HTTP2_STREAM_ERROR',
+      name: 'Error',
+      message: 'Stream closed with error code NGHTTP2_CANCEL'
+    }));
+    request.on('end', common.mustNotCall());
+    request.on('close', common.mustCall(function() {
       client.close();
     }));
     request.end();

test/parallel/test-http2-compat-socket-destroy-delayed.js

@@ -1,19 +1,14 @@
 'use strict';
 
 const common = require('../common');
-const { mustCall } = common;
+const { mustCall, mustNotCall } = common;
 
 if (!common.hasCrypto)
   common.skip('missing crypto');
 
 const http2 = require('http2');
 const assert = require('assert');
 
-const {
-  HTTP2_HEADER_PATH,
-  HTTP2_HEADER_METHOD,
-} = http2.constants;
-
 // This tests verifies that calling `req.socket.destroy()` via
 // setImmediate does not crash.
 // Fixes https://github.com/nodejs/node/issues/22855.
@@ -25,18 +20,22 @@ const app = http2.createServer(mustCall((req, res) => {
 
 app.listen(0, mustCall(() => {
   const session = http2.connect(`http://localhost:${app.address().port}`);
-  const request = session.request({
-    [HTTP2_HEADER_PATH]: '/',
-    [HTTP2_HEADER_METHOD]: 'get'
-  });
+  const request = session.request();
   request.once('response', mustCall((headers, flags) => {
     let data = '';
     request.on('data', (chunk) => { data += chunk; });
-    request.on('end', mustCall(() => {
+    // This should error since the server fails to finish the stream
+    request.on('error', mustCall((err) => {
+      common.expectsError({
+        code: 'ERR_HTTP2_STREAM_ERROR',
+        name: 'Error',
+        message: 'Stream closed with error code NGHTTP2_CANCEL'
+      })(err);
       assert.strictEqual(data, 'hello');
       session.close();
       app.close();
     }));
+    request.on('end', mustNotCall());
   }));
   request.end();
 }));

test/parallel/test-http2-compat-socket-set.js

@@ -83,7 +83,7 @@ server.on('request', common.mustCall(function(request, response) {
       assert.strictEqual(request.stream._isProcessing, true);
     });
   }));
-  response.stream.destroy();
+  response.end();
 }));
 
 server.listen(0, common.mustCall(function() {

test/parallel/test-http2-compat-socket.js

@@ -49,7 +49,7 @@ server.on('request', common.mustCall(function(request, response) {
 
   request.on('end', common.mustCall(() => {
     assert.strictEqual(request.socket.readable, false);
-    response.socket.destroy();
+    response.end();
   }));
   response.on('finish', common.mustCall(() => {
     assert.ok(request.socket);
@@ -84,7 +84,9 @@ server.listen(0, common.mustCall(function() {
       ':authority': `localhost:${port}`
     };
     const request = client.request(headers);
-    request.on('end', common.mustCall(() => {
+    request.on('error', common.mustNotCall());
+    request.on('end', common.mustCall());
+    request.on('close', common.mustCall(() => {
       client.close();
     }));
     request.end();

test/parallel/test-http2-compat-write-head-destroyed.js

@@ -19,8 +19,10 @@ const server = http2.createServer(common.mustCall((req, res) => {
 server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
 
-  const req = client.request();
+  const req = client.request({}, { endStream: false });
   req.on('response', common.mustNotCall());
+  req.on('error', common.mustNotCall());
+  req.on('aborted', common.mustCall());
   req.on('close', common.mustCall((arg) => {
     client.close();
     server.close();

test/parallel/test-http2-large-write-destroy.js

@@ -32,8 +32,12 @@ server.listen(0, common.mustCall(() => {
 
   const req = client.request({ ':path': '/' });
   req.end();
-  req.resume(); // Otherwise close won't be emitted if there's pending data.
 
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
   req.on('close', common.mustCall(() => {
     client.close();
     server.close();

test/parallel/test-http2-max-concurrent-streams.js

@@ -45,7 +45,7 @@ server.listen(0, common.mustCall(() => {
     req.on('aborted', common.mustCall());
     req.on('response', common.mustNotCall());
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => countdown.dec()));
     req.on('error', common.expectsError({
       code: 'ERR_HTTP2_STREAM_ERROR',

test/parallel/test-http2-multi-content-length.js

@@ -54,7 +54,7 @@ server.listen(0, common.mustCall(() => {
     // header to be set for non-payload bearing requests...
     const req = client.request({ 'content-length': 1 });
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => countdown.dec()));
     req.on('error', common.expectsError({
       code: 'ERR_HTTP2_STREAM_ERROR',

test/parallel/test-http2-respond-errors.js

@@ -43,7 +43,13 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
 
-  req.on('end', common.mustCall(() => {
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => {
     client.close();
     server.close();
   }));

test/parallel/test-http2-respond-file-errors.js

@@ -94,6 +94,12 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
 
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => {
     client.close();
     server.close();

test/parallel/test-http2-respond-file-fd-errors.js

@@ -117,6 +117,12 @@ server.listen(0, common.mustCall(() => {
   const client = http2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
 
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => {
     client.close();
     server.close();

test/parallel/test-http2-respond-file-fd-invalid.js

@@ -40,7 +40,8 @@ server.listen(0, () => {
   req.on('response', common.mustCall());
   req.on('error', errorCheck);
   req.on('data', common.mustNotCall());
-  req.on('end', common.mustCall(() => {
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => {
     assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
     client.close();
     server.close();

test/parallel/test-http2-respond-nghttperrors.js

@@ -88,7 +88,8 @@ function runTest(test) {
   req.resume();
   req.end();
 
-  req.on('end', common.mustCall(() => {
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => {
     client.close();
 
     if (!tests.length) {

test/parallel/test-http2-respond-with-fd-errors.js

@@ -96,7 +96,8 @@ function runTest(test) {
   req.resume();
   req.end();
 
-  req.on('end', common.mustCall(() => {
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => {
     client.close();
 
     if (!tests.length) {

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

@@ -28,5 +28,11 @@ server.listen(0, common.mustCall(() => {
     net.Socket.prototype.destroy.call(client.socket);
     server.close();
   }));
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
   req.end();
 }));

test/parallel/test-http2-server-shutdown-before-respond.js

@@ -32,5 +32,6 @@ server.on('listening', common.mustCall(() => {
   }));
   req.resume();
   req.on('data', common.mustNotCall());
-  req.on('end', common.mustCall(() => server.close()));
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => server.close()));
 }));

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

@@ -59,6 +59,12 @@ server.listen(
     const client = http2.connect(`http://localhost:${server.address().port}`);
     const req = client.request();
     req.resume();
+    req.on('error', common.expectsError({
+      code: 'ERR_HTTP2_STREAM_ERROR',
+      name: 'Error',
+      message: 'Stream closed with error code NGHTTP2_CANCEL'
+    }));
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => {
       client.close();
       server.close();

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

@@ -48,6 +48,11 @@ server.listen(0, common.mustCall(() => {
   const client = h2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
   req.resume();
-  req.on('end', common.mustCall());
+  req.on('error', common.expectsError({
+    code: 'ERR_HTTP2_STREAM_ERROR',
+    name: 'Error',
+    message: 'Stream closed with error code NGHTTP2_CANCEL'
+  }));
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => server.close(common.mustCall())));
 }));

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

@@ -17,10 +17,16 @@ server.on('stream', (stream, headers) => {
     '__proto__': null,
     [http2.sensitiveHeaders]: []
   });
-  stream.session.destroy();
+  stream.respond({ ':status': 200 });
+  stream.end();
   server.close();
 });
 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.mustNotCall());
+  req.on('end', common.mustCall());
+  req.on('close', common.mustCall(() => {
+    client.close();
+  }));
 }));

test/sequential/test-http2-max-session-memory.js

@@ -38,7 +38,7 @@ server.listen(0, common.mustCall(() => {
       }));
       req.on('close', common.mustCall(() => {
         server.close();
-        client.destroy();
+        client.close();
       }));
     });