FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: fixes error handling · nodejs/node@8403f00 · GitHub

/ node Public

Commit 8403f00

Browse files
mcollina
committed
http2: fixes error handling
There should be no default error handling when using Http2Stream. All errors will end up in `'streamError'` on the server anyway, but they are emitted on `'stream'` as well, otherwise some error conditions are impossible to debug. See: #14991 PR-URL: #19232 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 33e63fe commit 8403f00

6 files changed

Lines changed: 36 additions & 12 deletions

‎lib/internal/http2/core.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2073,19 +2073,12 @@ function afterOpen(session, options, headers, streamOptions, err, fd) {
20732073
headers, streamOptions));
20742074
}
20752075

2076-
function streamOnError(err) {
2077-
// we swallow the error for parity with HTTP1
2078-
// all the errors that ends here are not critical for the project
2079-
}
2080-
2081-
20822076
class ServerHttp2Stream extends Http2Stream {
20832077
constructor(session, handle, id, options, headers) {
20842078
super(session, options);
20852079
this[kInit](id, handle);
20862080
this[kProtocol] = headers[HTTP2_HEADER_SCHEME];
20872081
this[kAuthority] = headers[HTTP2_HEADER_AUTHORITY];
2088-
this.on('error', streamOnError);
20892082
}
20902083

20912084
// true if the remote peer accepts push streams

‎test/parallel/test-http2-misbehaving-multiplex.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,15 @@ const server = h2.createServer();
1616
server.on('stream', common.mustCall((stream) => {
1717
stream.respond();
1818
stream.end('ok');
19+
20+
// the error will be emitted asynchronously
21+
stream.on('error', common.expectsError({
22+
type: NghttpError,
23+
code: 'ERR_HTTP2_ERROR',
24+
message: 'Stream was already closed or invalid'
25+
}));
1926
}, 2));
27+
2028
server.on('session', common.mustCall((session) => {
2129
session.on('error', common.expectsError({
2230
code: 'ERR_HTTP2_ERROR',

‎test/parallel/test-http2-server-errors.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,16 @@ const h2 = require('http2');
5454

5555
const server = h2.createServer();
5656

57+
process.on('uncaughtException', common.mustCall(function(err) {
58+
assert.strictEqual(err.message, 'kaboom no handler');
59+
}));
60+
5761
server.on('stream', common.mustCall(function(stream) {
58-
// there is no 'error' handler, and this will not crash
62+
// there is no 'error' handler, and this will crash
5963
stream.write('hello');
6064
stream.resume();
6165

62-
expected = new Error('kaboom');
66+
expected = new Error('kaboom no handler');
6367
stream.destroy(expected);
6468
server.close();
6569
}));

‎test/parallel/test-http2-server-push-stream-head.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ server.on('stream', common.mustCall((stream, headers) => {
2121
}, common.mustCall((err, push, headers) => {
2222
assert.strictEqual(push._writableState.ended, true);
2323
push.respond();
24+
// cannot write to push() anymore
25+
push.on('error', common.expectsError({
26+
type: Error,
27+
code: 'ERR_STREAM_WRITE_AFTER_END',
28+
message: 'write after end'
29+
}));
2430
assert(!push.write('test'));
2531
stream.end('test');
2632
}));

‎test/parallel/test-http2-server-rst-stream.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,22 @@ const {
1818
const tests = [
1919
[NGHTTP2_NO_ERROR, false],
2020
[NGHTTP2_NO_ERROR, false],
21-
[NGHTTP2_PROTOCOL_ERROR, true],
21+
[NGHTTP2_PROTOCOL_ERROR, true, 'NGHTTP2_PROTOCOL_ERROR'],
2222
[NGHTTP2_CANCEL, false],
23-
[NGHTTP2_REFUSED_STREAM, true],
24-
[NGHTTP2_INTERNAL_ERROR, true]
23+
[NGHTTP2_REFUSED_STREAM, true, 'NGHTTP2_REFUSED_STREAM'],
24+
[NGHTTP2_INTERNAL_ERROR, true, 'NGHTTP2_INTERNAL_ERROR']
2525
];
2626

2727
const server = http2.createServer();
2828
server.on('stream', (stream, headers) => {
29+
const test = tests.find((t) => t[0] === Number(headers.rstcode));
30+
if (test[1]) {
31+
stream.on('error', common.expectsError({
32+
type: Error,
33+
code: 'ERR_HTTP2_STREAM_ERROR',
34+
message: `Stream closed with error code ${test[2]}`
35+
}));
36+
}
2937
stream.close(headers.rstcode | 0);
3038
});
3139

‎test/parallel/test-http2-server-stream-session-destroy.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ server.on('stream', common.mustCall((stream) => {
3434
type: Error
3535
}
3636
);
37+
stream.on('error', common.expectsError({
38+
type: Error,
39+
code: 'ERR_STREAM_WRITE_AFTER_END',
40+
message: 'write after end'
41+
}));
3742
assert.strictEqual(stream.write('data'), false);
3843
}));
3944

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL