test: http2 client destroy tests in one file

Refs: #14985
PR-URL: https://github.com/nodejs/node/pull/15749
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

lib/internal/http2/core.js

@@ -671,8 +671,6 @@ function submitShutdown(options) {
 
 function finishSessionDestroy(self, socket) {
   const state = self[kState];
-  if (state.destroyed)
-    return;
 
   if (!socket.destroyed)
     socket.destroy();
@@ -955,6 +953,7 @@ class Http2Session extends EventEmitter {
       return;
     debug(`[${sessionName(this[kType])}] destroying nghttp2session`);
     state.destroying = true;
+    state.destroyed = false;
 
     // Unenroll the timer
     this.setTimeout(0, sessionOnTimeout);
@@ -969,9 +968,6 @@ class Http2Session extends EventEmitter {
     delete this[kSocket];
     delete this[kServer];
 
-    state.destroyed = false;
-    state.destroying = true;
-
     if (this[kHandle] !== undefined)
       this[kHandle].destroying();
 

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

@@ -1,29 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-const h2 = require('http2');
-
-const server = h2.createServer();
-
-// we use the lower-level API here
-server.on('stream', common.mustNotCall());
-
-server.listen(0);
-
-server.on('listening', common.mustCall(() => {
-
-  const client = h2.connect(`http://localhost:${server.address().port}`);
-
-  const req = client.request({ ':path': '/' });
-  client.destroy();
-
-  req.on('response', common.mustNotCall());
-  req.resume();
-  req.on('end', common.mustCall(() => {
-    server.close();
-  }));
-  req.end();
-
-}));

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

@@ -1,29 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-const assert = require('assert');
-const h2 = require('http2');
-
-const server = h2.createServer();
-
-// we use the lower-level API here
-server.on('stream', common.mustNotCall());
-
-server.listen(0);
-
-server.on('listening', common.mustCall(() => {
-
-  const client = h2.connect(`http://localhost:${server.address().port}`);
-  client.destroy();
-
-  assert.throws(() => client.request({ ':path': '/' }),
-                common.expectsError({
-                  code: 'ERR_HTTP2_INVALID_SESSION',
-                  message: /^The session has been destroyed$/
-                }));
-
-  server.close();
-
-}));

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

@@ -1,24 +0,0 @@
-'use strict';
-
-const common = require('../common');
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-const http2 = require('http2');
-
-const server = http2.createServer();
-server.on('stream', common.mustCall((stream) => {
-  stream.on('error', common.mustCall());
-  stream.session.shutdown();
-}));
-
-server.listen(0, common.mustCall(() => {
-  const client = http2.connect(`http://localhost:${server.address().port}`);
-
-  client.on('goaway', common.mustCall(() => {
-    // We ought to be able to destroy the client in here without an error
-    server.close();
-    client.destroy();
-  }));
-
-  client.request();
-}));

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

@@ -6,50 +6,142 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const h2 = require('http2');
 
-const server = h2.createServer();
-server.listen(0);
+{
+  const server = h2.createServer();
+  server.listen(
+    0,
+    common.mustCall(() => {
+      const destroyCallbacks = [
+        (client) => client.destroy(),
+        (client) => client.socket.destroy()
+      ];
 
-server.on('listening', common.mustCall(function() {
-  const port = this.address().port;
+      let remaining = destroyCallbacks.length;
 
-  const destroyCallbacks = [
-    (client) => client.destroy(),
-    (client) => client.socket.destroy()
-  ];
+      destroyCallbacks.forEach((destroyCallback) => {
+        const client = h2.connect(`http://localhost:${server.address().port}`);
+        client.on(
+          'connect',
+          common.mustCall(() => {
+            const socket = client.socket;
 
-  let remaining = destroyCallbacks.length;
+            assert(client.socket, 'client session has associated socket');
+            assert(
+              !client.destroyed,
+              'client has not been destroyed before destroy is called'
+            );
+            assert(
+              !socket.destroyed,
+              'socket has not been destroyed before destroy is called'
+            );
 
-  destroyCallbacks.forEach((destroyCallback) => {
-    const client = h2.connect(`http://localhost:${port}`);
-    client.on('connect', common.mustCall(() => {
-      const socket = client.socket;
+            // Ensure that 'close' event is emitted
+            client.on('close', common.mustCall());
 
-      assert(client.socket, 'client session has associated socket');
-      assert(!client.destroyed,
-             'client has not been destroyed before destroy is called');
-      assert(!socket.destroyed,
-             'socket has not been destroyed before destroy is called');
+            destroyCallback(client);
 
-      // Ensure that 'close' event is emitted
-      client.on('close', common.mustCall());
+            assert(
+              !client.socket,
+              'client.socket undefined after destroy is called'
+            );
 
-      destroyCallback(client);
+            // Must must be closed
+            client.on(
+              'close',
+              common.mustCall(() => {
+                assert(client.destroyed);
+              })
+            );
 
-      assert(!client.socket, 'client.socket undefined after destroy is called');
+            // socket will close on process.nextTick
+            socket.on(
+              'close',
+              common.mustCall(() => {
+                assert(socket.destroyed);
+              })
+            );
 
-      // Must must be closed
-      client.on('close', common.mustCall(() => {
-        assert(client.destroyed);
-      }));
+            if (--remaining === 0) {
+              server.close();
+            }
+          })
+        );
+      });
+    })
+  );
+}
 
-      // socket will close on process.nextTick
-      socket.on('close', common.mustCall(() => {
-        assert(socket.destroyed);
-      }));
+// test destroy before connect
+{
+  const server = h2.createServer();
+  server.listen(
+    0,
+    common.mustCall(() => {
+      const client = h2.connect(`http://localhost:${server.address().port}`);
 
-      if (--remaining === 0) {
-        server.close();
-      }
-    }));
-  });
-}));
+      const req = client.request({ ':path': '/' });
+      client.destroy();
+
+      req.on('response', common.mustNotCall());
+      req.resume();
+      req.on(
+        'end',
+        common.mustCall(() => {
+          server.close();
+        })
+      );
+      req.end();
+    })
+  );
+}
+
+// test destroy before request
+{
+  const server = h2.createServer();
+  server.listen(
+    0,
+    common.mustCall(() => {
+      const client = h2.connect(`http://localhost:${server.address().port}`);
+      client.destroy();
+
+      assert.throws(
+        () => client.request({ ':path': '/' }),
+        common.expectsError({
+          code: 'ERR_HTTP2_INVALID_SESSION',
+          message: 'The session has been destroyed'
+        })
+      );
+
+      server.close();
+    })
+  );
+}
+
+// test destroy before goaway
+{
+  const server = h2.createServer();
+  server.on(
+    'stream',
+    common.mustCall((stream) => {
+      stream.on('error', common.mustCall());
+      stream.session.shutdown();
+    })
+  );
+  server.listen(
+    0,
+    common.mustCall(() => {
+      const client = h2.connect(`http://localhost:${server.address().port}`);
+
+      client.on(
+        'goaway',
+        common.mustCall(() => {
+          // We ought to be able to destroy the client in here without an error
+          server.close();
+          client.destroy();
+        })
+      );
+
+      client.request();
+    })
+  );
+}