http2: eliminate unnecessary nextTicks, use stream close event

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

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');
   }
 }
 
@@ -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;
 
@@ -1612,7 +1611,7 @@ class Http2Stream extends Duplex {
       }
     }
 
-    process.nextTick(emit, this, 'timeout');
+    this.emit('timeout');
   }
 
   // true if the HEADERS frame has been sent
@@ -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();
   }));