From ccd3afc84303247b32c011fea7630d150f07849d Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 30 Sep 2017 10:06:21 -0400 Subject: [PATCH] http2: adjust error emit in core, add tests Use the ability of nextTick and setImmediate to pass arguments instead of creating closures or binding. Add tests that cover the vast majority of error emits. PR-URL: https://github.com/nodejs/node/pull/15586 Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- lib/internal/http2/core.js | 207 +++++++++--------- .../test-http2-info-headers-errors.js | 105 +++++++++ test/parallel/test-http2-info-headers.js | 9 + test/parallel/test-http2-priority-errors.js | 109 +++++++++ test/parallel/test-http2-respond-errors.js | 104 +++++++++ .../test-http2-respond-file-errors.js | 31 ++- .../test-http2-respond-file-fd-errors.js | 40 +++- .../test-http2-respond-with-fd-errors.js | 109 +++++++++ test/parallel/test-http2-rststream-errors.js | 108 +++++++++ .../test-http2-server-push-stream-errors.js | 1 - test/parallel/test-http2-shutdown-errors.js | 75 +++++++ 11 files changed, 788 insertions(+), 110 deletions(-) create mode 100644 test/parallel/test-http2-info-headers-errors.js create mode 100644 test/parallel/test-http2-priority-errors.js create mode 100644 test/parallel/test-http2-respond-errors.js create mode 100644 test/parallel/test-http2-respond-with-fd-errors.js create mode 100644 test/parallel/test-http2-rststream-errors.js create mode 100644 test/parallel/test-http2-shutdown-errors.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index bc41cd4bf9..78a4896ab7 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -130,8 +130,8 @@ const { } = constants; // Top level to avoid creating a closure -function emit() { - this.emit.apply(this, arguments); +function emit(self, ...args) { + self.emit(...args); } // Called when a new block of headers has been received for a given @@ -169,7 +169,7 @@ function onSessionHeaders(id, cat, flags, headers) { stream = new ClientHttp2Stream(owner, id, { readable: !endOfStream }); } streams.set(id, stream); - process.nextTick(emit.bind(owner, 'stream', stream, obj, flags, headers)); + process.nextTick(emit, owner, 'stream', stream, obj, flags, headers); } else { let event; const status = obj[HTTP2_HEADER_STATUS]; @@ -192,7 +192,7 @@ function onSessionHeaders(id, cat, flags, headers) { } } debug(`[${sessionName(owner[kType])}] emitting stream '${event}' event`); - process.nextTick(emit.bind(stream, event, obj, flags, headers)); + process.nextTick(emit, stream, event, obj, flags, headers); } if (endOfStream) { stream.push(null); @@ -224,7 +224,7 @@ function onSessionTrailers(id) { getTrailers.call(stream, trailers); const headersList = mapToHeaders(trailers, assertValidPseudoHeaderTrailer); if (!Array.isArray(headersList)) { - process.nextTick(() => stream.emit('error', headersList)); + process.nextTick(emit, stream, 'error', headersList); return; } return headersList; @@ -258,14 +258,14 @@ function onSessionStreamClose(id, code) { function afterFDClose(err) { if (err) - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); } // Called when an error event needs to be triggered function onSessionError(error) { const owner = this[kOwner]; _unrefActive(owner); - process.nextTick(() => owner.emit('error', error)); + process.nextTick(emit, owner, 'error', error); } // Receives a chunk of data for a given stream and forwards it on @@ -314,7 +314,7 @@ function onSettings(ack) { if (owner.listenerCount(event) > 0) { const settings = event === 'localSettings' ? owner.localSettings : owner.remoteSettings; - process.nextTick(emit.bind(owner, event, settings)); + process.nextTick(emit, owner, event, settings); } } @@ -330,15 +330,14 @@ function onPriority(id, parent, weight, exclusive) { const streams = owner[kState].streams; const stream = streams.get(id); const emitter = stream === undefined ? owner : stream; - process.nextTick( - emit.bind(emitter, 'priority', id, parent, weight, exclusive)); + process.nextTick(emit, emitter, 'priority', id, parent, weight, exclusive); } -function emitFrameError(id, type, code) { - if (!this.emit('frameError', type, code, id)) { +function emitFrameError(self, id, type, code) { + if (!self.emit('frameError', type, code, id)) { const err = new errors.Error('ERR_HTTP2_FRAME_ERROR', type, code, id); err.errno = code; - this.emit('error', err); + self.emit('error', err); } } @@ -352,27 +351,27 @@ function onFrameError(id, type, code) { const streams = owner[kState].streams; const stream = streams.get(id); const emitter = stream !== undefined ? stream : owner; - process.nextTick(emitFrameError.bind(emitter, id, type, code)); + process.nextTick(emitFrameError, emitter, id, type, code); } -function emitGoaway(state, code, lastStreamID, buf) { - this.emit('goaway', code, lastStreamID, buf); +function emitGoaway(self, code, lastStreamID, buf) { + self.emit('goaway', code, lastStreamID, buf); // Tear down the session or destroy + const state = self[kState]; if (state.destroying || state.destroyed) return; if (!state.shuttingDown && !state.shutdown) { - this.shutdown({}, this.destroy.bind(this)); + self.shutdown({}, self.destroy.bind(self)); } else { - this.destroy(); + self.destroy(); } } // Called by the native layer when a goaway frame has been received function onGoawayData(code, lastStreamID, buf) { const owner = this[kOwner]; - const state = owner[kState]; debug(`[${sessionName(owner[kType])}] goaway data received`); - process.nextTick(emitGoaway.bind(owner, state, code, lastStreamID, buf)); + process.nextTick(emitGoaway, owner, code, lastStreamID, buf); } // Returns the padding to use per frame. The selectPadding callback is set @@ -408,7 +407,7 @@ function requestOnConnect(headers, options) { const headersList = mapToHeaders(headers); if (!Array.isArray(headersList)) { - process.nextTick(() => this.emit('error', headersList)); + process.nextTick(emit, this, 'error', headersList); return; } @@ -441,21 +440,21 @@ function requestOnConnect(headers, options) { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; case NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE: err = new errors.Error('ERR_HTTP2_OUT_OF_STREAMS'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; case NGHTTP2_ERR_INVALID_ARGUMENT: err = new errors.Error('ERR_HTTP2_STREAM_SELF_DEPENDENCY'); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; default: // Some other, unexpected error was returned. Emit on the session. if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; } debug(`[${sessionName(session[kType])}] stream ${ret} initialized`); @@ -542,7 +541,7 @@ function setupHandle(session, socket, type, options) { options.settings : Object.create(null); session.settings(settings); - process.nextTick(emit.bind(session, 'connect', session, socket)); + process.nextTick(emit, session, 'connect', session, socket); }; } @@ -559,13 +558,13 @@ function submitSettings(settings) { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; default: // Some other unexpected error was reported. if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); } } debug(`[${sessionName(this[kType])}] settings complete`); @@ -591,13 +590,13 @@ function submitPriority(stream, options) { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; default: // Some other unexpected error was reported. if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, stream, 'error', err); } } debug(`[${sessionName(this[kType])}] priority complete`); @@ -615,13 +614,13 @@ function submitRstStream(stream, code) { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; default: // Some other unexpected error was reported. if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, stream, 'error', err); break; } stream.destroy(); @@ -629,9 +628,9 @@ function submitRstStream(stream, code) { debug(`[${sessionName(this[kType])}] rststream complete`); } -function doShutdown(options) { - const handle = this[kHandle]; - const state = this[kState]; +function doShutdown(self, options) { + const handle = self[kHandle]; + const state = self[kState]; if (handle === undefined || state.shutdown) return; // Nothing to do, possibly because the session shutdown already. const ret = handle.submitGoaway(options.errorCode | 0, @@ -640,13 +639,13 @@ function doShutdown(options) { state.shuttingDown = false; state.shutdown = true; if (ret < 0) { - debug(`[${sessionName(this[kType])}] shutdown failed! code: ${ret}`); + debug(`[${sessionName(self[kType])}] shutdown failed! code: ${ret}`); const err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, self, 'error', err); return; } - process.nextTick(emit.bind(this, 'shutdown', options)); - debug(`[${sessionName(this[kType])}] shutdown is complete`); + process.nextTick(emit, self, 'shutdown', options); + debug(`[${sessionName(self[kType])}] shutdown is complete`); } // Submit a graceful or immediate shutdown request for the Http2Session. @@ -659,14 +658,14 @@ function submitShutdown(options) { // first send a shutdown notice handle.submitShutdownNotice(); // then, on flip of the event loop, do the actual shutdown - setImmediate(doShutdown.bind(this, options)); + setImmediate(doShutdown, this, options); } else { - doShutdown.call(this, options); + doShutdown(this, options); } } -function finishSessionDestroy(socket) { - const state = this[kState]; +function finishSessionDestroy(self, socket) { + const state = self[kState]; if (state.destroyed) return; @@ -677,15 +676,15 @@ function finishSessionDestroy(socket) { state.destroyed = true; // Destroy the handle - const handle = this[kHandle]; + const handle = self[kHandle]; if (handle !== undefined) { handle.destroy(state.skipUnconsume); - debug(`[${sessionName(this[kType])}] nghttp2session handle destroyed`); + debug(`[${sessionName(self[kType])}] nghttp2session handle destroyed`); } - this[kHandle] = undefined; + self[kHandle] = undefined; - process.nextTick(emit.bind(this, 'close')); - debug(`[${sessionName(this[kType])}] nghttp2session destroyed`); + process.nextTick(emit, self, 'close'); + debug(`[${sessionName(self[kType])}] nghttp2session destroyed`); } // Upon creation, the Http2Session takes ownership of the socket. The session @@ -971,7 +970,7 @@ class Http2Session extends EventEmitter { if (this[kHandle] !== undefined) this[kHandle].destroying(); - setImmediate(finishSessionDestroy.bind(this, socket)); + setImmediate(finishSessionDestroy, this, socket); } // Graceful or immediate shutdown of the Http2Session. Graceful shutdown @@ -1039,7 +1038,7 @@ class Http2Session extends EventEmitter { } _onTimeout() { - process.nextTick(emit.bind(this, 'timeout')); + process.nextTick(emit, this, 'timeout'); } } @@ -1168,7 +1167,7 @@ function onHandleFinish() { const session = this[kSession]; if (session === undefined) return; if (this[kID] === undefined) { - this.once('ready', onHandleFinish.bind(this)); + this.once('ready', onHandleFinish); } else { const handle = session[kHandle]; if (handle !== undefined) { @@ -1201,7 +1200,7 @@ function streamOnResume() { if (this._paused) return this.pause(); if (this[kID] === undefined) { - this.once('ready', streamOnResume.bind(this)); + this.once('ready', streamOnResume); return; } const session = this[kSession]; @@ -1238,7 +1237,7 @@ function streamOnSessionConnect() { debug(`[${sessionName(session[kType])}] session connected. emiting stream ` + 'connect'); this[kState].connecting = false; - process.nextTick(emit.bind(this, 'connect')); + process.nextTick(emit, this, 'connect'); } function streamOnceReady() { @@ -1320,7 +1319,7 @@ class Http2Stream extends Duplex { } _onTimeout() { - process.nextTick(emit.bind(this, 'timeout')); + process.nextTick(emit, this, 'timeout'); } // true if the Http2Stream was aborted abornomally. @@ -1485,7 +1484,6 @@ class Http2Stream extends Duplex { // * Then cleans up the resources on the js side _destroy(err, callback) { const session = this[kSession]; - const handle = session[kHandle]; if (this[kID] === undefined) { debug(`[${sessionName(session[kType])}] queuing destroy for new stream`); this.once('ready', this._destroy.bind(this, err, callback)); @@ -1498,47 +1496,53 @@ class Http2Stream extends Duplex { server.emit('streamError', err, this); } - process.nextTick(() => { - debug(`[${sessionName(session[kType])}] destroying stream ${this[kID]}`); + process.nextTick(continueStreamDestroy, this, err, callback); + } +} - // Submit RST-STREAM frame if one hasn't been sent already and the - // stream hasn't closed normally... - if (!this[kState].rst && !session.destroyed) { - const code = - err instanceof Error ? - NGHTTP2_INTERNAL_ERROR : NGHTTP2_NO_ERROR; - this[kSession].rstStream(this, code); - } +function continueStreamDestroy(self, err, callback) { + const session = self[kSession]; + const handle = session[kHandle]; + const state = self[kState]; - // Remove the close handler on the session - session.removeListener('close', this[kState].closeHandler); + debug(`[${sessionName(session[kType])}] destroying stream ${self[kID]}`); - // Unenroll the timer - this.setTimeout(0); + // Submit RST-STREAM frame if one hasn't been sent already and the + // stream hasn't closed normally... + if (!state.rst && !session.destroyed) { + const code = + err instanceof Error ? + NGHTTP2_INTERNAL_ERROR : NGHTTP2_NO_ERROR; + session.rstStream(self, code); + } - setImmediate(finishStreamDestroy.bind(this, handle)); + // Remove the close handler on the session + session.removeListener('close', state.closeHandler); - // All done - const rst = this[kState].rst; - const code = rst ? this[kState].rstCode : NGHTTP2_NO_ERROR; - if (!err && code !== NGHTTP2_NO_ERROR) { - err = new errors.Error('ERR_HTTP2_STREAM_ERROR', code); - } - process.nextTick(emit.bind(this, 'streamClosed', code)); - debug(`[${sessionName(session[kType])}] stream ${this[kID]} destroyed`); - callback(err); - }); + // Unenroll the timer + self.setTimeout(0); + + setImmediate(finishStreamDestroy, self, handle); + + // All done + const rst = state.rst; + const code = rst ? state.rstCode : NGHTTP2_NO_ERROR; + if (!err && code !== NGHTTP2_NO_ERROR) { + err = new errors.Error('ERR_HTTP2_STREAM_ERROR', code); } + process.nextTick(emit, self, 'streamClosed', code); + debug(`[${sessionName(session[kType])}] stream ${self[kID]} destroyed`); + callback(err); } -function finishStreamDestroy(handle) { - const id = this[kID]; - const session = this[kSession]; +function finishStreamDestroy(self, handle) { + const id = self[kID]; + const session = self[kSession]; session[kState].streams.delete(id); - delete this[kSession]; + delete self[kSession]; if (handle !== undefined) handle.destroyStream(id); - this.emit('destroy'); + self.emit('destroy'); } function processHeaders(headers) { @@ -1578,12 +1582,12 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1, switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; default: if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); } } } @@ -1594,7 +1598,7 @@ function doSendFD(session, options, fd, headers, getTrailers, err, stat) { return; } if (err) { - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); return; } @@ -1611,7 +1615,8 @@ function doSendFD(session, options, fd, headers, getTrailers, err, stat) { const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); if (!Array.isArray(headersList)) { - process.nextTick(() => this.emit('error', headersList)); + process.nextTick(emit, this, 'error', headersList); + return; } processRespondWithFD.call(this, fd, headersList, @@ -1668,7 +1673,8 @@ function doSendFileFD(session, options, fd, headers, getTrailers, err, stat) { const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); if (!Array.isArray(headersList)) { - process.nextTick(() => this.emit('error', headersList)); + process.nextTick(emit, this, 'error', headersList); + return; } processRespondWithFD.call(this, fd, headersList, @@ -1786,20 +1792,20 @@ class ServerHttp2Stream extends Http2Stream { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; case NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE: err = new errors.Error('ERR_HTTP2_OUT_OF_STREAMS'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; case NGHTTP2_ERR_STREAM_CLOSED: err = new errors.Error('ERR_HTTP2_STREAM_CLOSED'); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; default: if (ret <= 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); break; } debug(`[${sessionName(session[kType])}] push stream ${ret} created`); @@ -1882,12 +1888,12 @@ class ServerHttp2Stream extends Http2Stream { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => session.emit('error', err)); + process.nextTick(emit, session, 'error', err); break; default: if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); } } } @@ -1963,7 +1969,8 @@ class ServerHttp2Stream extends Http2Stream { const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); if (!Array.isArray(headersList)) { - process.nextTick(() => this.emit('error', headersList)); + process.nextTick(emit, this, 'error', headersList); + return; } processRespondWithFD.call(this, fd, headersList, @@ -2079,12 +2086,12 @@ class ServerHttp2Stream extends Http2Stream { switch (ret) { case NGHTTP2_ERR_NOMEM: err = new errors.Error('ERR_OUTOFMEMORY'); - process.nextTick(() => this[kSession].emit('error', err)); + process.nextTick(emit, this[kSession], 'error', err); break; default: if (ret < 0) { err = new NghttpError(ret); - process.nextTick(() => this.emit('error', err)); + process.nextTick(emit, this, 'error', err); } } } @@ -2289,7 +2296,7 @@ function connectionListener(socket) { socket[kServer] = this; - process.nextTick(emit.bind(this, 'session', session)); + process.nextTick(emit, this, 'session', session); } function initializeOptions(options) { diff --git a/test/parallel/test-http2-info-headers-errors.js b/test/parallel/test-http2-info-headers-errors.js new file mode 100644 index 0000000000..5e1c2d1fad --- /dev/null +++ b/test/parallel/test-http2-info-headers-errors.js @@ -0,0 +1,105 @@ +'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 additionalHeaders +// - NGHTTP2_ERR_NOMEM (should emit session error) +// - every other NGHTTP2 error from binding (should emit stream error) + +const specificTestKeys = [ + 'NGHTTP2_ERR_NOMEM' +]; + +const specificTests = [ + { + ngError: constants.NGHTTP2_ERR_NOMEM, + error: { + code: 'ERR_OUTOFMEMORY', + type: Error, + message: 'Out of memory' + }, + type: 'session' + } +]; + +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 sendHeaders because we only care about testing error handling +Http2Session.prototype.sendHeaders = () => 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}` + ); + + 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.additionalHeaders({ ':status': 100 }); +}, 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-info-headers.js b/test/parallel/test-http2-info-headers.js index 1ef6f244e2..609f56e8b8 100644 --- a/test/parallel/test-http2-info-headers.js +++ b/test/parallel/test-http2-info-headers.js @@ -32,6 +32,15 @@ function onStream(stream, headers, flags) { message: status101regex })); + common.expectsError( + () => stream.additionalHeaders({ ':method': 'POST' }), + { + code: 'ERR_HTTP2_INVALID_PSEUDOHEADER', + type: Error, + message: '":method" is an invalid pseudoheader or is used incorrectly' + } + ); + // Can send more than one stream.additionalHeaders({ ':status': 100 }); stream.additionalHeaders({ ':status': 100 }); diff --git a/test/parallel/test-http2-priority-errors.js b/test/parallel/test-http2-priority-errors.js new file mode 100644 index 0000000000..d29d2f72fa --- /dev/null +++ b/test/parallel/test-http2-priority-errors.js @@ -0,0 +1,109 @@ +'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 priority +// - NGHTTP2_ERR_NOMEM (should emit session error) +// - every other NGHTTP2 error from binding (should emit stream error) + +const specificTestKeys = [ + 'NGHTTP2_ERR_NOMEM' +]; + +const specificTests = [ + { + ngError: constants.NGHTTP2_ERR_NOMEM, + error: { + code: 'ERR_OUTOFMEMORY', + type: Error, + message: 'Out of memory' + }, + type: 'session' + } +]; + +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 submitPriority because we only care about testing error handling +Http2Session.prototype.submitPriority = () => 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}` + ); + + 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.priority({ + parent: 0, + weight: 1, + exclusive: false + }); +}, 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-respond-errors.js b/test/parallel/test-http2-respond-errors.js new file mode 100644 index 0000000000..4e2c39178e --- /dev/null +++ b/test/parallel/test-http2-respond-errors.js @@ -0,0 +1,104 @@ +'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 respond +// - NGHTTP2_ERR_NOMEM (should emit session error) +// - every other NGHTTP2 error from binding (should emit stream error) + +const specificTestKeys = [ + 'NGHTTP2_ERR_NOMEM' +]; + +const specificTests = [ + { + ngError: constants.NGHTTP2_ERR_NOMEM, + error: { + code: 'ERR_OUTOFMEMORY', + type: Error, + message: 'Out of memory' + }, + type: 'session' + } +]; + +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 submitResponse because we only care about testing error handling +Http2Session.prototype.submitResponse = () => 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}` + ); + + if (currentError.type === 'stream') { + stream.session.on('error', errorMustNotCall); + stream.on('error', errorMustCall); + stream.on('error', common.mustCall(() => { + stream.destroy(); + })); + } else { + stream.session.once('error', errorMustCall); + stream.on('error', errorMustNotCall); + } + + stream.respond(); +}, 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-respond-file-errors.js b/test/parallel/test-http2-respond-file-errors.js index d5f071b376..ac2e8f3498 100644 --- a/test/parallel/test-http2-respond-file-errors.js +++ b/test/parallel/test-http2-respond-file-errors.js @@ -6,6 +6,11 @@ if (!common.hasCrypto) const http2 = require('http2'); const path = require('path'); +const { + HTTP2_HEADER_CONTENT_TYPE, + HTTP2_HEADER_METHOD +} = http2.constants; + const optionsWithTypeError = { offset: 'number', length: 'number', @@ -54,7 +59,7 @@ server.on('stream', common.mustCall((stream) => { // Should throw if :status 204, 205 or 304 [204, 205, 304].forEach((status) => common.expectsError( () => stream.respondWithFile(fname, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain', + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain', ':status': status, }), { @@ -63,13 +68,31 @@ server.on('stream', common.mustCall((stream) => { } )); + // should emit an error on the stream if headers aren't valid + stream.respondWithFile(fname, { + [HTTP2_HEADER_METHOD]: 'POST' + }, { + statCheck: common.mustCall(() => { + // give time to the current test case to finish + process.nextTick(continueTest, stream); + return true; + }) + }); + stream.once('error', common.expectsError({ + code: 'ERR_HTTP2_INVALID_PSEUDOHEADER', + type: Error, + message: '":method" is an invalid pseudoheader or is used incorrectly' + })); +})); + +function continueTest(stream) { // Should throw if headers already sent stream.respond({ ':status': 200, }); common.expectsError( () => stream.respondWithFile(fname, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }), { code: 'ERR_HTTP2_HEADERS_SENT', @@ -81,14 +104,14 @@ server.on('stream', common.mustCall((stream) => { stream.destroy(); common.expectsError( () => stream.respondWithFile(fname, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }), { code: 'ERR_HTTP2_INVALID_STREAM', message: 'The stream has been destroyed' } ); -})); +} server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); diff --git a/test/parallel/test-http2-respond-file-fd-errors.js b/test/parallel/test-http2-respond-file-fd-errors.js index faf37f3373..95c4b5fea6 100644 --- a/test/parallel/test-http2-respond-file-fd-errors.js +++ b/test/parallel/test-http2-respond-file-fd-errors.js @@ -7,6 +7,11 @@ const http2 = require('http2'); const path = require('path'); const fs = require('fs'); +const { + HTTP2_HEADER_CONTENT_TYPE, + HTTP2_HEADER_METHOD +} = http2.constants; + const optionsWithTypeError = { offset: 'number', length: 'number', @@ -38,7 +43,7 @@ server.on('stream', common.mustCall((stream) => { common.expectsError( () => stream.respondWithFD(types[type], { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }), { type: TypeError, @@ -57,7 +62,7 @@ server.on('stream', common.mustCall((stream) => { common.expectsError( () => stream.respondWithFD(fd, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }, { [option]: types[type] }), @@ -74,25 +79,49 @@ server.on('stream', common.mustCall((stream) => { // Should throw if :status 204, 205 or 304 [204, 205, 304].forEach((status) => common.expectsError( () => stream.respondWithFD(fd, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain', + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain', ':status': status, }), { code: 'ERR_HTTP2_PAYLOAD_FORBIDDEN', + type: Error, message: `Responses with ${status} status must not have a payload` } )); + // should emit an error on the stream if headers aren't valid + stream.respondWithFD(fd, { + [HTTP2_HEADER_METHOD]: 'POST' + }, { + statCheck() { + return true; + } + }); + stream.once('error', common.expectsError({ + code: 'ERR_HTTP2_INVALID_PSEUDOHEADER', + type: Error, + message: '":method" is an invalid pseudoheader or is used incorrectly' + })); + stream.respondWithFD(fd, { + [HTTP2_HEADER_METHOD]: 'POST' + }); + stream.once('error', common.expectsError({ + code: 'ERR_HTTP2_INVALID_PSEUDOHEADER', + type: Error, + message: '":method" is an invalid pseudoheader or is used incorrectly' + })); + // Should throw if headers already sent stream.respond({ ':status': 200, }); common.expectsError( () => stream.respondWithFD(fd, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }), { code: 'ERR_HTTP2_HEADERS_SENT', + type: Error, message: 'Response has already been initiated.' } ); @@ -101,10 +130,11 @@ server.on('stream', common.mustCall((stream) => { stream.destroy(); common.expectsError( () => stream.respondWithFD(fd, { - [http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' + [HTTP2_HEADER_CONTENT_TYPE]: 'text/plain' }), { code: 'ERR_HTTP2_INVALID_STREAM', + type: Error, message: 'The stream has been destroyed' } ); diff --git a/test/parallel/test-http2-respond-with-fd-errors.js b/test/parallel/test-http2-respond-with-fd-errors.js new file mode 100644 index 0000000000..8ec0d9bf71 --- /dev/null +++ b/test/parallel/test-http2-respond-with-fd-errors.js @@ -0,0 +1,109 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); +const path = require('path'); + +const { + constants, + Http2Session, + nghttp2ErrorString +} = process.binding('http2'); + +// tests error handling within processRespondWithFD +// (called by respondWithFD & respondWithFile) +// - NGHTTP2_ERR_NOMEM (should emit session error) +// - every other NGHTTP2 error from binding (should emit stream error) + +const fname = path.resolve(common.fixturesDir, 'elipses.txt'); + +const specificTestKeys = [ + 'NGHTTP2_ERR_NOMEM' +]; + +const specificTests = [ + { + ngError: constants.NGHTTP2_ERR_NOMEM, + error: { + code: 'ERR_OUTOFMEMORY', + type: Error, + message: 'Out of memory' + }, + type: 'session' + } +]; + +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 submitFile because we only care about testing error handling +Http2Session.prototype.submitFile = () => 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}` + ); + + if (currentError.type === 'stream') { + stream.session.on('error', errorMustNotCall); + stream.on('error', errorMustCall); + stream.on('error', common.mustCall(() => { + stream.destroy(); + })); + } else { + stream.session.once('error', errorMustCall); + stream.on('error', errorMustNotCall); + } + + stream.respondWithFile(fname); +}, 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-rststream-errors.js b/test/parallel/test-http2-rststream-errors.js new file mode 100644 index 0000000000..58d4440f2e --- /dev/null +++ b/test/parallel/test-http2-rststream-errors.js @@ -0,0 +1,108 @@ +'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 rstStream +// - NGHTTP2_ERR_NOMEM (should emit session error) +// - every other NGHTTP2 error from binding (should emit stream error) + +const specificTestKeys = [ + 'NGHTTP2_ERR_NOMEM' +]; + +const specificTests = [ + { + ngError: constants.NGHTTP2_ERR_NOMEM, + error: { + code: 'ERR_OUTOFMEMORY', + type: Error, + message: 'Out of memory' + }, + type: 'session' + } +]; + +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 submitRstStream because we only care about testing error handling +Http2Session.prototype.submitRstStream = () => 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}` + ); + + if (currentError.type === 'stream') { + stream.session.on('error', errorMustNotCall); + stream.on('error', errorMustCall); + stream.on('error', common.mustCall(() => { + stream.session.destroy(); + })); + } else { + stream.session.once('error', errorMustCall); + stream.on('error', errorMustNotCall); + } + + stream.rstStream(); +}, 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(); + + if (currentError.type === 'stream') { + req.on('error', common.mustCall()); + } + + 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-errors.js b/test/parallel/test-http2-server-push-stream-errors.js index 42d58c2ca1..777b20eb3f 100644 --- a/test/parallel/test-http2-server-push-stream-errors.js +++ b/test/parallel/test-http2-server-push-stream-errors.js @@ -81,7 +81,6 @@ server.on('stream', common.mustCall((stream, headers) => { const errorMustNotCall = common.mustNotCall( `${currentError.error.code} should emit on ${currentError.type}` ); - console.log(currentError); if (currentError.type === 'stream') { stream.session.on('error', errorMustNotCall); diff --git a/test/parallel/test-http2-shutdown-errors.js b/test/parallel/test-http2-shutdown-errors.js new file mode 100644 index 0000000000..99ae791767 --- /dev/null +++ b/test/parallel/test-http2-shutdown-errors.js @@ -0,0 +1,75 @@ +'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 shutdown +// - should emit ERR_HTTP2_ERROR on session for all errors + +const tests = Object.getOwnPropertyNames(constants) + .filter((key) => ( + key.indexOf('NGHTTP2_ERR') === 0 + )) + .map((key) => ({ + ngError: constants[key], + error: { + code: 'ERR_HTTP2_ERROR', + type: Error, + message: nghttp2ErrorString(constants[key]) + } + })); + +let currentError; + +// mock submitGoaway because we only care about testing error handling +Http2Session.prototype.submitGoaway = () => 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 session` + ); + + stream.session.once('error', errorMustCall); + stream.on('error', errorMustNotCall); + + stream.session.shutdown(); +}, 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()); + } + })); +}