http2: add tests for push stream error handling

Add tests that cover errors for wrong arguments, as well as
tests for error codes from nghttp2. Fix pushStream to emit
NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE on session rather than
stream.

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

5 files changed, 244 insertions, 3 deletions

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 5a0e12b414..eb6b605f67 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -1814,7 +1814,7 @@ class ServerHttp2Stream extends Http2Stream {
         break;
       case NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE:
         err = new errors.Error('ERR_HTTP2_OUT_OF_STREAMS');
-        process.nextTick(() => this.emit('error', err));
+        process.nextTick(() => session.emit('error', err));
         break;
       case NGHTTP2_ERR_STREAM_CLOSED:
         err = new errors.Error('ERR_HTTP2_STREAM_CLOSED');
diff --git a/test/parallel/test-http2-server-push-stream-errors-args.js b/test/parallel/test-http2-server-push-stream-errors-args.js
new file mode 100644
index 0000000000..7d0c104594
--- /dev/null
+++ b/test/parallel/test-http2-server-push-stream-errors-args.js
@@ -0,0 +1,57 @@
+// Flags: --expose-http2
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+// Check that pushStream handles being passed wrong arguments
+// in the expected manner
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream, headers) => {
+  const port = server.address().port;
+
+  // Must receive a callback (function)
+  common.expectsError(
+    () => stream.pushStream({
+      ':scheme': 'http',
+      ':path': '/foobar',
+      ':authority': `localhost:${port}`,
+    }, {}, 'callback'),
+    {
+      code: 'ERR_INVALID_CALLBACK',
+      message: 'Callback must be a function'
+    }
+  );
+
+  // Must validate headers
+  common.expectsError(
+    () => stream.pushStream({ 'connection': 'test' }, {}, () => {}),
+    {
+      code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
+      message: 'HTTP/1 Connection specific headers are forbidden'
+    }
+  );
+
+  stream.end('test');
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const headers = { ':path': '/' };
+  const client = http2.connect(`http://localhost:${port}`);
+  const req = client.request(headers);
+  req.setEncoding('utf8');
+
+  let data = '';
+  req.on('data', common.mustCall((d) => data += d));
+  req.on('end', common.mustCall(() => {
+    assert.strictEqual(data, 'test');
+    server.close();
+    client.destroy();
+  }));
+  req.end();
+}));
diff --git a/test/parallel/test-http2-server-push-stream-errors.js b/test/parallel/test-http2-server-push-stream-errors.js
new file mode 100644
index 0000000000..ad26874f8a
--- /dev/null
+++ b/test/parallel/test-http2-server-push-stream-errors.js
@@ -0,0 +1,130 @@
+// Flags: --expose-http2
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+const {
+  constants,
+  Http2Session,
+  nghttp2ErrorString
+} = process.binding('http2');
+
+// tests error handling within pushStream
+// - NGHTTP2_ERR_NOMEM (should emit session error)
+// - NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE (should emit session error)
+// - NGHTTP2_ERR_STREAM_CLOSED (should emit stream error)
+// - every other NGHTTP2 error from binding (should emit stream error)
+
+const specificTestKeys = [
+  'NGHTTP2_ERR_NOMEM',
+  'NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE',
+  'NGHTTP2_ERR_STREAM_CLOSED'
+];
+
+const specificTests = [
+  {
+    ngError: constants.NGHTTP2_ERR_NOMEM,
+    error: {
+      code: 'ERR_OUTOFMEMORY',
+      type: Error,
+      message: 'Out of memory'
+    },
+    type: 'session'
+  },
+  {
+    ngError: constants.NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE,
+    error: {
+      code: 'ERR_HTTP2_OUT_OF_STREAMS',
+      type: Error,
+      message: 'No stream ID is available because ' +
+               'maximum stream ID has been reached'
+    },
+    type: 'session'
+  },
+  {
+    ngError: constants.NGHTTP2_ERR_STREAM_CLOSED,
+    error: {
+      code: 'ERR_HTTP2_STREAM_CLOSED',
+      type: Error,
+      message: 'The stream is already closed'
+    },
+    type: 'stream'
+  },
+];
+
+const genericTests = Object.getOwnPropertyNames(constants)
+  .filter((key) => (
+    key.indexOf('NGHTTP2_ERR') === 0 && specificTestKeys.indexOf(key) < 0
+  ))
+  .map((key) => ({
+    ngError: constants[key],
+    error: {
+      code: 'ERR_HTTP2_ERROR',
+      type: Error,
+      message: nghttp2ErrorString(constants[key])
+    },
+    type: 'stream'
+  }));
+
+
+const tests = specificTests.concat(genericTests);
+
+let currentError;
+
+// mock submitPushPromise because we only care about testing error handling
+Http2Session.prototype.submitPushPromise = () => currentError.ngError;
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream, headers) => {
+  const errorMustCall = common.expectsError(currentError.error);
+  const errorMustNotCall = common.mustNotCall(
+    `${currentError.error.code} should emit on ${currentError.type}`
+  );
+  console.log(currentError);
+
+  if (currentError.type === 'stream') {
+    stream.session.on('error', errorMustNotCall);
+    stream.on('error', errorMustCall);
+    stream.on('error', common.mustCall(() => {
+      stream.respond();
+      stream.end();
+    }));
+  } else {
+    stream.session.once('error', errorMustCall);
+    stream.on('error', errorMustNotCall);
+  }
+
+  stream.pushStream({}, () => {});
+}, tests.length));
+
+server.listen(0, common.mustCall(() => runTest(tests.shift())));
+
+function runTest(test) {
+  const port = server.address().port;
+  const url = `http://localhost:${port}`;
+  const headers = {
+    ':path': '/',
+    ':method': 'POST',
+    ':scheme': 'http',
+    ':authority': `localhost:${port}`
+  };
+
+  const client = http2.connect(url);
+  const req = client.request(headers);
+
+  currentError = test;
+  req.resume();
+  req.end();
+
+  req.on('end', common.mustCall(() => {
+    client.destroy();
+
+    if (!tests.length) {
+      server.close();
+    } else {
+      runTest(tests.shift());
+    }
+  }));
+}
diff --git a/test/parallel/test-http2-server-push-stream-head.js b/test/parallel/test-http2-server-push-stream-head.js
new file mode 100644
index 0000000000..a1172b8ecf
--- /dev/null
+++ b/test/parallel/test-http2-server-push-stream-head.js
@@ -0,0 +1,54 @@
+// Flags: --expose-http2
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+// Check that pushStream handles method HEAD correctly
+// - stream should end immediately (no body)
+
+const server = http2.createServer();
+server.on('stream', common.mustCall((stream, headers) => {
+  const port = server.address().port;
+  if (headers[':path'] === '/') {
+    stream.pushStream({
+      ':scheme': 'http',
+      ':method': 'HEAD',
+      ':authority': `localhost:${port}`,
+    }, common.mustCall((push, headers) => {
+      assert.strictEqual(push._writableState.ended, true);
+      stream.end('test');
+    }));
+  }
+  stream.respond({
+    'content-type': 'text/html',
+    ':status': 200
+  });
+}));
+
+server.listen(0, common.mustCall(() => {
+  const port = server.address().port;
+  const headers = { ':path': '/' };
+  const client = http2.connect(`http://localhost:${port}`);
+  const req = client.request(headers);
+  req.setEncoding('utf8');
+
+  client.on('stream', common.mustCall((stream, headers) => {
+    assert.strictEqual(headers[':scheme'], 'http');
+    assert.strictEqual(headers[':path'], '/');
+    assert.strictEqual(headers[':authority'], `localhost:${port}`);
+  }));
+
+  let data = '';
+
+  req.on('data', common.mustCall((d) => data += d));
+  req.on('end', common.mustCall(() => {
+    assert.strictEqual(data, 'test');
+    server.close();
+    client.destroy();
+  }));
+  req.end();
+}));
diff --git a/test/parallel/test-http2-server-push-stream.js b/test/parallel/test-http2-server-push-stream.js
index c1de1195f7..c79ff7caec 100644
--- a/test/parallel/test-http2-server-push-stream.js
+++ b/test/parallel/test-http2-server-push-stream.js
@@ -15,7 +15,7 @@ server.on('stream', common.mustCall((stream, headers) => {
       ':scheme': 'http',
       ':path': '/foobar',
       ':authority': `localhost:${port}`,
-    }, (push, headers) => {
+    }, common.mustCall((push, headers) => {
       push.respond({
         'content-type': 'text/html',
         ':status': 200,
@@ -23,7 +23,7 @@ server.on('stream', common.mustCall((stream, headers) => {
       });
       push.end('pushed by server data');
       stream.end('test');
-    });
+    }));
   }
   stream.respond({
     'content-type': 'text/html',