http2: do not emit our own close emit in Http2Stream

Streams were recently updated to emit their own close event. The
Http2Stream was an exception because it included the close argument
with the close event. Refactor that to use the built in close.

PR-URL: #19451
Reviewed-By: Matteo Collina <[email protected]>

Author: jasnell

doc/api/http2.md

@@ -335,7 +335,7 @@ Immediately terminates the `Http2Session` and the associated `net.Socket` or
 `tls.TLSSocket`.
 
 Once destroyed, the `Http2Session` will emit the `'close'` event. If `error`
-is not undefined, an `'error'` event will be emitted immediately after the
+is not undefined, an `'error'` event will be emitted immediately before the
 `'close'` event.
 
 If there are any remaining open `Http2Streams` associated with the
@@ -816,9 +816,9 @@ added: v8.4.0
 The `'close'` event is emitted when the `Http2Stream` is destroyed. Once
 this event is emitted, the `Http2Stream` instance is no longer usable.
 
-The listener callback is passed a single argument specifying the HTTP/2 error
-code specified when closing the stream. If the code is any value other than
-`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will also be emitted.
+The HTTP/2 error code used when closing the stream can be retrieved using
+the `http2stream.rstCode` property. If the code is any value other than
+`NGHTTP2_NO_ERROR` (`0`), an `'error'` event will have also been emitted.
 
 #### Event: 'error'
 <!-- YAML

lib/internal/http2/core.js

@@ -1503,7 +1503,6 @@ class Http2Stream extends Duplex {
   constructor(session, options) {
     options.allowHalfOpen = true;
     options.decodeStrings = false;
-    options.emitClose = false;
     super(options);
     this[async_id_symbol] = -1;
 
@@ -1888,9 +1887,7 @@ class Http2Stream extends Duplex {
     // will destroy if it has been closed and there are no other open or
     // pending streams.
     session[kMaybeDestroy]();
-    process.nextTick(emit, this, 'close', code);
     callback(err);
-
   }
   // The Http2Stream can be destroyed if it has closed and if the readable
   // side has received the final chunk.

test/parallel/test-http2-client-rststream-before-connect.js

@@ -49,9 +49,9 @@ server.listen(0, common.mustCall(() => {
   // Second call doesn't do anything.
   req.close(closeCode + 1);
 
-  req.on('close', common.mustCall((code) => {
+  req.on('close', common.mustCall(() => {
     assert.strictEqual(req.destroyed, true);
-    assert.strictEqual(code, closeCode);
+    assert.strictEqual(req.rstCode, closeCode);
     server.close();
     client.close();
   }));

test/parallel/test-http2-client-stream-destroy-before-connect.js

@@ -36,9 +36,9 @@ server.listen(0, common.mustCall(() => {
     message: 'test'
   }));
 
-  req.on('close', common.mustCall((code) => {
+  req.on('close', common.mustCall(() => {
+    assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
     assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
-    assert.strictEqual(code, NGHTTP2_INTERNAL_ERROR);
     server.close();
     client.close();
   }));

test/parallel/test-http2-options-max-reserved-streams.js

@@ -37,8 +37,8 @@ server.on('stream', common.mustCall((stream) => {
     pushedStream.respond();
     pushedStream.on('aborted', common.mustCall());
     pushedStream.on('error', common.mustNotCall());
-    pushedStream.on('close', common.mustCall((code) => {
-      assert.strictEqual(code, 8);
+    pushedStream.on('close', common.mustCall(() => {
+      assert.strictEqual(pushedStream.rstCode, 8);
       countdown.dec();
     }));
   }));

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

@@ -28,8 +28,8 @@ server.on('listening', common.mustCall(() => {
   const client = h2.connect(`http://localhost:${server.address().port}`);
   const req = client.request();
   req.on('headers', common.mustNotCall());
-  req.on('close', common.mustCall((code) => {
-    assert.strictEqual(h2.constants.NGHTTP2_NO_ERROR, code);
+  req.on('close', common.mustCall(() => {
+    assert.strictEqual(h2.constants.NGHTTP2_NO_ERROR, req.rstCode);
     server.close();
     client.close();
   }));

test/parallel/test-http2-server-rst-stream.js

@@ -50,8 +50,8 @@ server.listen(0, common.mustCall(() => {
       ':method': 'POST',
       'rstcode': test[0]
     });
-    req.on('close', common.mustCall((code) => {
-      assert.strictEqual(code, test[0]);
+    req.on('close', common.mustCall(() => {
+      assert.strictEqual(req.rstCode, test[0]);
       countdown.dec();
     }));
     req.on('aborted', common.mustCall());

test/parallel/test-http2-too-large-headers.js

@@ -22,8 +22,8 @@ server.listen(0, common.mustCall(() => {
       type: Error,
       message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
     }));
-    req.on('close', common.mustCall((code) => {
-      assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
+    req.on('close', common.mustCall(() => {
+      assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
       server.close();
       client.close();
     }));

test/parallel/test-http2-too-many-headers.js

@@ -25,8 +25,8 @@ server.listen(0, common.mustCall(() => {
     type: Error,
     message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM'
   }));
-  req.on('close', common.mustCall((code) => {
-    assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM);
+  req.on('close', common.mustCall(() => {
+    assert.strictEqual(req.rstCode, NGHTTP2_ENHANCE_YOUR_CALM);
     server.close();
     client.close();
   }));