From d3b9090d068e35e38e3ec3a9709859646eb8eb35 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 19 Mar 2018 08:17:26 -0700
Subject: [PATCH 1/2] http2: remove some unnecessary next ticks

---
 lib/internal/http2/core.js | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 9315474364f59c..fc73eccdad92c9 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -409,7 +409,7 @@ function onSettings() {
   session[kUpdateTimer]();
   debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
   session[kRemoteSettings] = undefined;
-  process.nextTick(emit, session, 'remoteSettings', session.remoteSettings);
+  session.emit('remoteSettings', session.remoteSettings);
 }
 
 // If the stream exists, an attempt will be made to emit an event
@@ -425,7 +425,7 @@ function onPriority(id, parent, weight, exclusive) {
   const emitter = session[kState].streams.get(id) || session;
   if (!emitter.destroyed) {
     emitter[kUpdateTimer]();
-    process.nextTick(emit, emitter, 'priority', id, parent, weight, exclusive);
+    emitter.emit('priority', id, parent, weight, exclusive);
   }
 }
 
@@ -439,7 +439,7 @@ function onFrameError(id, type, code) {
         `type ${type} on stream ${id}, code: ${code}`);
   const emitter = session[kState].streams.get(id) || session;
   emitter[kUpdateTimer]();
-  process.nextTick(emit, emitter, 'frameError', type, code, id);
+  emitter.emit('frameError', type, code, id);
 }
 
 function onAltSvc(stream, origin, alt) {
@@ -449,7 +449,7 @@ function onAltSvc(stream, origin, alt) {
   debug(`Http2Session ${sessionName(session[kType])}: altsvc received: ` +
         `stream: ${stream}, origin: ${origin}, alt: ${alt}`);
   session[kUpdateTimer]();
-  process.nextTick(emit, session, 'altsvc', alt, origin, stream);
+  session.emit('altsvc', alt, origin, stream);
 }
 
 // Receiving a GOAWAY frame from the connected peer is a signal that no
@@ -771,7 +771,7 @@ function setupHandle(socket, type, options) {
   // core will check for session.destroyed before progressing, this
   // ensures that those at l`east get cleared out.
   if (this.destroyed) {
-    process.nextTick(emit, this, 'connect', this, socket);
+    this.emit('connect', this, socket);
     return;
   }
   debug(`Http2Session ${sessionName(type)}: setting up session handle`);
@@ -813,7 +813,7 @@ function setupHandle(socket, type, options) {
     options.settings : {};
 
   this.settings(settings);
-  process.nextTick(emit, this, 'connect', this, socket);
+  this.emit('connect', this, socket);
 }
 
 // Emits a close event followed by an error event if err is truthy. Used
@@ -1262,7 +1262,7 @@ class Http2Session extends EventEmitter {
       }
     }
 
-    process.nextTick(emit, this, 'timeout');
+    this.emit('timeout');
   }
 
   ref() {
@@ -1485,8 +1485,8 @@ function streamOnPause() {
 function abort(stream) {
   if (!stream.aborted &&
       !(stream._writableState.ended || stream._writableState.ending)) {
-    process.nextTick(emit, stream, 'aborted');
     stream[kState].flags |= STREAM_FLAGS_ABORTED;
+    stream.emit('aborted');
   }
 }
 
@@ -1612,7 +1612,7 @@ class Http2Stream extends Duplex {
       }
     }
 
-    process.nextTick(emit, this, 'timeout');
+    this.emit('timeout');
   }
 
   // true if the HEADERS frame has been sent

From 744b268a71a37b5afdc68a8807e2ae35c90a3a92 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 2/2] 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.
---
 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 13aa4d1e2103d1..a27b27658a2ce0 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -332,7 +332,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
@@ -813,9 +813,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 fc73eccdad92c9..2485a145519494 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 547ad2fa43b10b..3120a80b11ddf4 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 884a38fac6f71d..d834de5d11ebe7 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 994a8817451686..6dbf54bf31c980 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 c2831d33079616..e7218aaeea29af 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 402ff07fa9d0a0..132c32ead600dd 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 ad7a24f39b2fa2..b62a41d05f5984 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 9b57060be695bb..6c8315c80ebf2f 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();
   }));