Skip to content

Commit a29cd25

Browse files
addaleaxrvagg
addaleax
authored and
rvagg
committed
http2: refer to stream errors by name
Display the constant name instead of a stream error code in the error message, because the numerical codes give absolutely no clue about what happened when an error is emitted. PR-URL: #18966 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent 21cdb73 commit a29cd25

19 files changed

Lines changed: 58 additions & 43 deletions

‎lib/internal/http2/core.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ const {
6262
} = require('timers');
6363

6464
const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
65-
const { constants } = binding;
65+
const { constants, nameForErrorCode } = binding;
6666

6767
const NETServer = net.Server;
6868
const TLSServer = tls.Server;
@@ -1842,7 +1842,8 @@ class Http2Stream extends Duplex {
18421842
// abort and is already covered by aborted event, also allows more
18431843
// seamless compatibility with http1
18441844
if (err == null && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL)
1845-
err = new errors.Error('ERR_HTTP2_STREAM_ERROR', code);
1845+
err = new errors.Error('ERR_HTTP2_STREAM_ERROR',
1846+
nameForErrorCode[code] || code);
18461847

18471848
this[kSession] = undefined;
18481849
this[kHandle] = undefined;

‎src/node_http2.cc‎

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,29 +2961,39 @@ void Initialize(Local<Object> target,
29612961
session->GetFunction()).FromJust();
29622962

29632963
Local<Object> constants = Object::New(isolate);
2964-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_SESSION_SERVER);
2965-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_SESSION_CLIENT);
2966-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_IDLE);
2967-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_OPEN);
2968-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_RESERVED_LOCAL);
2969-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_RESERVED_REMOTE);
2970-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_HALF_CLOSED_LOCAL);
2971-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_HALF_CLOSED_REMOTE);
2972-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_CLOSED);
2973-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_NO_ERROR);
2974-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_PROTOCOL_ERROR);
2975-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_INTERNAL_ERROR);
2976-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLOW_CONTROL_ERROR);
2977-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_TIMEOUT);
2978-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_CLOSED);
2979-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_FRAME_SIZE_ERROR);
2980-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_REFUSED_STREAM);
2981-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_CANCEL);
2982-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_COMPRESSION_ERROR);
2983-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_CONNECT_ERROR);
2984-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_ENHANCE_YOUR_CALM);
2985-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_INADEQUATE_SECURITY);
2986-
NODE_DEFINE_CONSTANT(constants, NGHTTP2_HTTP_1_1_REQUIRED);
2964+
Local<Array> name_for_error_code = Array::New(isolate);
2965+
2966+
#define NODE_NGHTTP2_ERROR_CODES(V) \
2967+
V(NGHTTP2_SESSION_SERVER); \
2968+
V(NGHTTP2_SESSION_CLIENT); \
2969+
V(NGHTTP2_STREAM_STATE_IDLE); \
2970+
V(NGHTTP2_STREAM_STATE_OPEN); \
2971+
V(NGHTTP2_STREAM_STATE_RESERVED_LOCAL); \
2972+
V(NGHTTP2_STREAM_STATE_RESERVED_REMOTE); \
2973+
V(NGHTTP2_STREAM_STATE_HALF_CLOSED_LOCAL); \
2974+
V(NGHTTP2_STREAM_STATE_HALF_CLOSED_REMOTE); \
2975+
V(NGHTTP2_STREAM_STATE_CLOSED); \
2976+
V(NGHTTP2_NO_ERROR); \
2977+
V(NGHTTP2_PROTOCOL_ERROR); \
2978+
V(NGHTTP2_INTERNAL_ERROR); \
2979+
V(NGHTTP2_FLOW_CONTROL_ERROR); \
2980+
V(NGHTTP2_SETTINGS_TIMEOUT); \
2981+
V(NGHTTP2_STREAM_CLOSED); \
2982+
V(NGHTTP2_FRAME_SIZE_ERROR); \
2983+
V(NGHTTP2_REFUSED_STREAM); \
2984+
V(NGHTTP2_CANCEL); \
2985+
V(NGHTTP2_COMPRESSION_ERROR); \
2986+
V(NGHTTP2_CONNECT_ERROR); \
2987+
V(NGHTTP2_ENHANCE_YOUR_CALM); \
2988+
V(NGHTTP2_INADEQUATE_SECURITY); \
2989+
V(NGHTTP2_HTTP_1_1_REQUIRED); \
2990+
2991+
#define V(name) \
2992+
NODE_DEFINE_CONSTANT(constants, name); \
2993+
name_for_error_code->Set(static_cast<int>(name), \
2994+
FIXED_ONE_BYTE_STRING(isolate, #name));
2995+
NODE_NGHTTP2_ERROR_CODES(V)
2996+
#undef V
29872997

29882998
NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_REQUEST);
29892999
NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_RESPONSE);
@@ -3048,6 +3058,9 @@ HTTP_STATUS_CODES(V)
30483058
target->Set(context,
30493059
FIXED_ONE_BYTE_STRING(isolate, "constants"),
30503060
constants).FromJust();
3061+
target->Set(context,
3062+
FIXED_ONE_BYTE_STRING(isolate, "nameForErrorCode"),
3063+
name_for_error_code).FromJust();
30513064
}
30523065
} // namespace http2
30533066
} // namespace node

‎test/parallel/test-http2-client-rststream-before-connect.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ server.listen(0, common.mustCall(() => {
5959
req.on('error', common.expectsError({
6060
code: 'ERR_HTTP2_STREAM_ERROR',
6161
type: Error,
62-
message: `Stream closed with error code ${closeCode}`
62+
message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR'
6363
}));
6464

6565
req.on('response', common.mustCall());

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ server.on('stream', (stream) => {
1818
// system specific timings.
1919
stream.on('error', (err) => {
2020
assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR');
21-
assert.strictEqual(err.message, 'Stream closed with error code 2');
21+
assert.strictEqual(err.message,
22+
'Stream closed with error code NGHTTP2_INTERNAL_ERROR');
2223
});
2324
stream.respond();
2425
stream.end();

‎test/parallel/test-http2-client-unescaped-path.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ server.listen(0, common.mustCall(() => {
2727
req.on('error', common.expectsError({
2828
code: 'ERR_HTTP2_STREAM_ERROR',
2929
type: Error,
30-
message: 'Stream closed with error code 1'
30+
message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR'
3131
}));
3232
req.on('close', common.mustCall(() => countdown.dec()));
3333
}

‎test/parallel/test-http2-compat-serverresponse-destroy.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ server.listen(0, common.mustCall(() => {
5959
req.on('error', common.expectsError({
6060
code: 'ERR_HTTP2_STREAM_ERROR',
6161
type: Error,
62-
message: 'Stream closed with error code 2'
62+
message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
6363
}));
6464
req.on('close', common.mustCall(() => countdown.dec()));
6565

@@ -74,7 +74,7 @@ server.listen(0, common.mustCall(() => {
7474
req.on('error', common.expectsError({
7575
code: 'ERR_HTTP2_STREAM_ERROR',
7676
type: Error,
77-
message: 'Stream closed with error code 2'
77+
message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
7878
}));
7979
req.on('close', common.mustCall(() => countdown.dec()));
8080

‎test/parallel/test-http2-info-headers-errors.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ function runTest(test) {
7070
req.on('error', common.expectsError({
7171
code: 'ERR_HTTP2_STREAM_ERROR',
7272
type: Error,
73-
message: 'Stream closed with error code 2'
73+
message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
7474
}));
7575

7676
req.on('close', common.mustCall(() => {

‎test/parallel/test-http2-max-concurrent-streams.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ server.listen(0, common.mustCall(() => {
5050
req.on('error', common.expectsError({
5151
code: 'ERR_HTTP2_STREAM_ERROR',
5252
type: Error,
53-
message: 'Stream closed with error code 7'
53+
message: 'Stream closed with error code NGHTTP2_REFUSED_STREAM'
5454
}));
5555
}
5656
}));

‎test/parallel/test-http2-misbehaving-flow-control-paused.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ server.on('stream', (stream) => {
6363
stream.on('error', common.expectsError({
6464
code: 'ERR_HTTP2_STREAM_ERROR',
6565
type: Error,
66-
message: 'Stream closed with error code 3'
66+
message: 'Stream closed with error code NGHTTP2_FLOW_CONTROL_ERROR'
6767
}));
6868
stream.on('close', common.mustCall(() => {
6969
server.close();

‎test/parallel/test-http2-misbehaving-flow-control.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ server.on('stream', (stream) => {
6969
stream.on('error', common.expectsError({
7070
code: 'ERR_HTTP2_STREAM_ERROR',
7171
type: Error,
72-
message: 'Stream closed with error code 3'
72+
message: 'Stream closed with error code NGHTTP2_FLOW_CONTROL_ERROR'
7373
}));
7474
stream.on('close', common.mustCall(() => {
7575
server.close(common.mustCall());

0 commit comments

Comments
 (0)