From c2b861cb7497f74bb358d710293ef3099f377374 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 23 Aug 2017 18:29:49 +0200 Subject: [PATCH] http2: refactor error handling This changes the error handling model of ServerHttp2Stream, ServerHttp2Request and ServerHttp2Response. An 'error' emitted on ServerHttp2Stream will not go to 'uncaughtException' anymore, but to the server 'streamError'. On the stream 'error', ServerHttp2Request will emit 'abort', while ServerHttp2Response would do nothing. It also updates respondWith* to the new error handling. Fixes: https://github.com/nodejs/node/issues/14963 PR-URL: https://github.com/nodejs/node/pull/14991 Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum --- doc/api/http2.md | 39 +++++++- lib/http2.js | 4 +- lib/internal/http2/compat.js | 16 ++- lib/internal/http2/core.js | 43 ++++++-- ...p2-client-stream-destroy-before-connect.js | 17 +--- test/parallel/test-http2-compat-errors.js | 52 ++++++++++ test/parallel/test-http2-respond-file-404.js | 48 +++++++++ .../test-http2-respond-file-error-dir.js | 46 +++++++++ test/parallel/test-http2-server-errors.js | 97 +++++++++++++++++++ 9 files changed, 329 insertions(+), 33 deletions(-) create mode 100644 test/parallel/test-http2-compat-errors.js create mode 100644 test/parallel/test-http2-respond-file-404.js create mode 100644 test/parallel/test-http2-respond-file-error-dir.js create mode 100644 test/parallel/test-http2-server-errors.js diff --git a/doc/api/http2.md b/doc/api/http2.md index d9b8aa97b0..57e1b74e7e 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1118,6 +1118,8 @@ added: v8.4.0 * `headers` {[Headers Object][]} * `options` {Object} * `statCheck` {Function} + * `onError` {Function} Callback function invoked in the case of an + Error before send * `getTrailers` {Function} Callback function invoked to collect trailer headers. * `offset` {number} The offset position at which to begin reading @@ -1146,6 +1148,16 @@ server.on('stream', (stream) => { function statCheck(stat, headers) { headers['last-modified'] = stat.mtime.toUTCString(); } + + function onError(err) { + if (err.code === 'ENOENT') { + stream.respond({ ':status': 404 }); + } else { + stream.respond({ ':status': 500 }); + } + stream.end(); + } + stream.respondWithFile('/some/file', { 'content-type': 'text/plain' }, { statCheck }); @@ -1178,6 +1190,10 @@ The `offset` and `length` options may be used to limit the response to a specific range subset. This can be used, for instance, to support HTTP Range requests. +The `options.onError` function may also be used to handle all the errors +that could happen before the delivery of the file is initiated. The +default behavior is to destroy the stream. + When set, the `options.getTrailers()` function is called immediately after queuing the last chunk of payload data to be sent. The callback is passed a single object (with a `null` prototype) that the listener may used to specify @@ -1208,6 +1224,19 @@ added: v8.4.0 * Extends: {net.Server} +In `Http2Server`, there is no `'clientError'` event as there is in +HTTP1. However, there are `'socketError'`, `'sessionError'`, and +`'streamError'`, for error happened on the socket, session or stream +respectively. + +#### Event: 'socketError' + + +The `'socketError'` event is emitted when a `'socketError'` event is emitted by +an `Http2Session` associated with the server. + #### Event: 'sessionError' -The `'socketError'` event is emitted when a `'socketError'` event is emitted by -an `Http2Session` associated with the server. +* `socket` {http2.ServerHttp2Stream} + +If an `ServerHttp2Stream` emits an `'error'` event, it will be forwarded here. +The stream will already be destroyed when this event is triggered. #### Event: 'stream'