From cef909797af1d716b802f04264e51cf7f26c1b51 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 19 Mar 2018 09:17:23 -0700
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/19451
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/http2.md                                          | 8 ++++----
 lib/internal/http2/core.js                                | 3 ---
 .../test-http2-client-rststream-before-connect.js         | 4 ++--
 .../test-http2-client-stream-destroy-before-connect.js    | 4 ++--
 test/parallel/test-http2-options-max-reserved-streams.js  | 4 ++--
 test/parallel/test-http2-server-rst-before-respond.js     | 4 ++--
 test/parallel/test-http2-server-rst-stream.js             | 4 ++--
 test/parallel/test-http2-too-large-headers.js             | 4 ++--
 test/parallel/test-http2-too-many-headers.js              | 4 ++--
 9 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 484a681eae1..050ef5d0b22 100644
--- a/doc/api/http2.md
+++ b/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
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index fc73eccdad9..2485a145519 100644
--- a/lib/internal/http2/core.js
+++ b/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.
diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js
index 547ad2fa43b..3120a80b11d 100644
--- a/test/parallel/test-http2-client-rststream-before-connect.js
+++ b/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();
   }));
diff --git a/test/parallel/test-http2-client-stream-destroy-before-connect.js b/test/parallel/test-http2-client-stream-destroy-before-connect.js
index 884a38fac6f..d834de5d11e 100644
--- a/test/parallel/test-http2-client-stream-destroy-before-connect.js
+++ b/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();
   }));
diff --git a/test/parallel/test-http2-options-max-reserved-streams.js b/test/parallel/test-http2-options-max-reserved-streams.js
index 994a8817451..6dbf54bf31c 100644
--- a/test/parallel/test-http2-options-max-reserved-streams.js
+++ b/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();
     }));
   }));
diff --git a/test/parallel/test-http2-server-rst-before-respond.js b/test/parallel/test-http2-server-rst-before-respond.js
index c2831d33079..e7218aaeea2 100644
--- a/test/parallel/test-http2-server-rst-before-respond.js
+++ b/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();
   }));
diff --git a/test/parallel/test-http2-server-rst-stream.js b/test/parallel/test-http2-server-rst-stream.js
index 402ff07fa9d..132c32ead60 100644
--- a/test/parallel/test-http2-server-rst-stream.js
+++ b/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());
diff --git a/test/parallel/test-http2-too-large-headers.js b/test/parallel/test-http2-too-large-headers.js
index ad7a24f39b2..b62a41d05f5 100644
--- a/test/parallel/test-http2-too-large-headers.js
+++ b/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();
     }));
diff --git a/test/parallel/test-http2-too-many-headers.js b/test/parallel/test-http2-too-many-headers.js
index 9b57060be69..6c8315c80eb 100644
--- a/test/parallel/test-http2-too-many-headers.js
+++ b/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();
   }));