From a29cd25b41fc6862a930954e081da516650a87a8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 23 Feb 2018 22:28:29 +0100 Subject: [PATCH] http2: refer to stream errors by name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/18966 Reviewed-By: James M Snell Reviewed-By: Minwoo Jung Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen --- lib/internal/http2/core.js | 5 +- src/node_http2.cc | 59 +++++++++++-------- ...t-http2-client-rststream-before-connect.js | 2 +- ...p2-client-stream-destroy-before-connect.js | 3 +- .../test-http2-client-unescaped-path.js | 2 +- ...est-http2-compat-serverresponse-destroy.js | 4 +- .../test-http2-info-headers-errors.js | 2 +- .../test-http2-max-concurrent-streams.js | 2 +- ...t-http2-misbehaving-flow-control-paused.js | 2 +- .../test-http2-misbehaving-flow-control.js | 2 +- .../test-http2-misused-pseudoheaders.js | 2 +- .../test-http2-multi-content-length.js | 2 +- ...-http2-options-max-headers-block-length.js | 2 +- .../test-http2-respond-file-fd-invalid.js | 2 +- .../test-http2-respond-nghttperrors.js | 2 +- .../test-http2-respond-with-fd-errors.js | 2 +- test/parallel/test-http2-too-large-headers.js | 2 +- test/parallel/test-http2-too-many-headers.js | 2 +- .../test-http2-max-session-memory.js | 2 +- 19 files changed, 58 insertions(+), 43 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 1149caa84277e2..1afaa00126c997 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -62,7 +62,7 @@ const { } = require('timers'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); -const { constants } = binding; +const { constants, nameForErrorCode } = binding; const NETServer = net.Server; const TLSServer = tls.Server; @@ -1842,7 +1842,8 @@ class Http2Stream extends Duplex { // abort and is already covered by aborted event, also allows more // seamless compatibility with http1 if (err == null && code !== NGHTTP2_NO_ERROR && code !== NGHTTP2_CANCEL) - err = new errors.Error('ERR_HTTP2_STREAM_ERROR', code); + err = new errors.Error('ERR_HTTP2_STREAM_ERROR', + nameForErrorCode[code] || code); this[kSession] = undefined; this[kHandle] = undefined; diff --git a/src/node_http2.cc b/src/node_http2.cc index c5328cc4f4d233..e7681214a88ee2 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2961,29 +2961,39 @@ void Initialize(Local target, session->GetFunction()).FromJust(); Local constants = Object::New(isolate); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_SESSION_SERVER); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_SESSION_CLIENT); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_IDLE); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_OPEN); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_RESERVED_LOCAL); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_RESERVED_REMOTE); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_HALF_CLOSED_LOCAL); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_HALF_CLOSED_REMOTE); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_STATE_CLOSED); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_NO_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_PROTOCOL_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_INTERNAL_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_FLOW_CONTROL_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_TIMEOUT); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_STREAM_CLOSED); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_FRAME_SIZE_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_REFUSED_STREAM); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_CANCEL); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_COMPRESSION_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_CONNECT_ERROR); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_ENHANCE_YOUR_CALM); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_INADEQUATE_SECURITY); - NODE_DEFINE_CONSTANT(constants, NGHTTP2_HTTP_1_1_REQUIRED); + Local name_for_error_code = Array::New(isolate); + +#define NODE_NGHTTP2_ERROR_CODES(V) \ + V(NGHTTP2_SESSION_SERVER); \ + V(NGHTTP2_SESSION_CLIENT); \ + V(NGHTTP2_STREAM_STATE_IDLE); \ + V(NGHTTP2_STREAM_STATE_OPEN); \ + V(NGHTTP2_STREAM_STATE_RESERVED_LOCAL); \ + V(NGHTTP2_STREAM_STATE_RESERVED_REMOTE); \ + V(NGHTTP2_STREAM_STATE_HALF_CLOSED_LOCAL); \ + V(NGHTTP2_STREAM_STATE_HALF_CLOSED_REMOTE); \ + V(NGHTTP2_STREAM_STATE_CLOSED); \ + V(NGHTTP2_NO_ERROR); \ + V(NGHTTP2_PROTOCOL_ERROR); \ + V(NGHTTP2_INTERNAL_ERROR); \ + V(NGHTTP2_FLOW_CONTROL_ERROR); \ + V(NGHTTP2_SETTINGS_TIMEOUT); \ + V(NGHTTP2_STREAM_CLOSED); \ + V(NGHTTP2_FRAME_SIZE_ERROR); \ + V(NGHTTP2_REFUSED_STREAM); \ + V(NGHTTP2_CANCEL); \ + V(NGHTTP2_COMPRESSION_ERROR); \ + V(NGHTTP2_CONNECT_ERROR); \ + V(NGHTTP2_ENHANCE_YOUR_CALM); \ + V(NGHTTP2_INADEQUATE_SECURITY); \ + V(NGHTTP2_HTTP_1_1_REQUIRED); \ + +#define V(name) \ + NODE_DEFINE_CONSTANT(constants, name); \ + name_for_error_code->Set(static_cast(name), \ + FIXED_ONE_BYTE_STRING(isolate, #name)); + NODE_NGHTTP2_ERROR_CODES(V) +#undef V NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_REQUEST); NODE_DEFINE_HIDDEN_CONSTANT(constants, NGHTTP2_HCAT_RESPONSE); @@ -3048,6 +3058,9 @@ HTTP_STATUS_CODES(V) target->Set(context, FIXED_ONE_BYTE_STRING(isolate, "constants"), constants).FromJust(); + target->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "nameForErrorCode"), + name_for_error_code).FromJust(); } } // namespace http2 } // namespace node diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js index aeb31949db074e..72374611b49349 100644 --- a/test/parallel/test-http2-client-rststream-before-connect.js +++ b/test/parallel/test-http2-client-rststream-before-connect.js @@ -59,7 +59,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: `Stream closed with error code ${closeCode}` + message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' })); req.on('response', common.mustCall()); diff --git a/test/parallel/test-http2-client-stream-destroy-before-connect.js b/test/parallel/test-http2-client-stream-destroy-before-connect.js index a2412b9f1d646a..884a38fac6f71d 100644 --- a/test/parallel/test-http2-client-stream-destroy-before-connect.js +++ b/test/parallel/test-http2-client-stream-destroy-before-connect.js @@ -18,7 +18,8 @@ server.on('stream', (stream) => { // system specific timings. stream.on('error', (err) => { assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR'); - assert.strictEqual(err.message, 'Stream closed with error code 2'); + assert.strictEqual(err.message, + 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'); }); stream.respond(); stream.end(); diff --git a/test/parallel/test-http2-client-unescaped-path.js b/test/parallel/test-http2-client-unescaped-path.js index 190f8ce75e8917..ff122a02ca1a68 100644 --- a/test/parallel/test-http2-client-unescaped-path.js +++ b/test/parallel/test-http2-client-unescaped-path.js @@ -27,7 +27,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 1' + message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' })); req.on('close', common.mustCall(() => countdown.dec())); } diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js index 54214737840061..b528a64f79b20d 100644 --- a/test/parallel/test-http2-compat-serverresponse-destroy.js +++ b/test/parallel/test-http2-compat-serverresponse-destroy.js @@ -59,7 +59,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); req.on('close', common.mustCall(() => countdown.dec())); @@ -74,7 +74,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); req.on('close', common.mustCall(() => countdown.dec())); diff --git a/test/parallel/test-http2-info-headers-errors.js b/test/parallel/test-http2-info-headers-errors.js index 555b22242664ae..dbc8a5c44100f5 100644 --- a/test/parallel/test-http2-info-headers-errors.js +++ b/test/parallel/test-http2-info-headers-errors.js @@ -70,7 +70,7 @@ function runTest(test) { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); req.on('close', common.mustCall(() => { diff --git a/test/parallel/test-http2-max-concurrent-streams.js b/test/parallel/test-http2-max-concurrent-streams.js index ffc04e98f134b2..b270d6cc6aff31 100644 --- a/test/parallel/test-http2-max-concurrent-streams.js +++ b/test/parallel/test-http2-max-concurrent-streams.js @@ -50,7 +50,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 7' + message: 'Stream closed with error code NGHTTP2_REFUSED_STREAM' })); } })); diff --git a/test/parallel/test-http2-misbehaving-flow-control-paused.js b/test/parallel/test-http2-misbehaving-flow-control-paused.js index d69e0fd802979a..60a2cdabf847d9 100644 --- a/test/parallel/test-http2-misbehaving-flow-control-paused.js +++ b/test/parallel/test-http2-misbehaving-flow-control-paused.js @@ -63,7 +63,7 @@ server.on('stream', (stream) => { stream.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 3' + message: 'Stream closed with error code NGHTTP2_FLOW_CONTROL_ERROR' })); stream.on('close', common.mustCall(() => { server.close(); diff --git a/test/parallel/test-http2-misbehaving-flow-control.js b/test/parallel/test-http2-misbehaving-flow-control.js index 161a88ea1fb407..f2da0ba56c8e67 100644 --- a/test/parallel/test-http2-misbehaving-flow-control.js +++ b/test/parallel/test-http2-misbehaving-flow-control.js @@ -69,7 +69,7 @@ server.on('stream', (stream) => { stream.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 3' + message: 'Stream closed with error code NGHTTP2_FLOW_CONTROL_ERROR' })); stream.on('close', common.mustCall(() => { server.close(common.mustCall()); diff --git a/test/parallel/test-http2-misused-pseudoheaders.js b/test/parallel/test-http2-misused-pseudoheaders.js index fc53d01a2f6bb0..16b3fcd6a48552 100644 --- a/test/parallel/test-http2-misused-pseudoheaders.js +++ b/test/parallel/test-http2-misused-pseudoheaders.js @@ -42,7 +42,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); req.on('response', common.mustCall()); diff --git a/test/parallel/test-http2-multi-content-length.js b/test/parallel/test-http2-multi-content-length.js index 4d18356f127da0..c64b803a0ecca3 100644 --- a/test/parallel/test-http2-multi-content-length.js +++ b/test/parallel/test-http2-multi-content-length.js @@ -58,7 +58,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 1' + message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' })); } })); diff --git a/test/parallel/test-http2-options-max-headers-block-length.js b/test/parallel/test-http2-options-max-headers-block-length.js index a728c28c6576d4..0706e6a83e6d48 100644 --- a/test/parallel/test-http2-options-max-headers-block-length.js +++ b/test/parallel/test-http2-options-max-headers-block-length.js @@ -38,6 +38,6 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 7' + message: 'Stream closed with error code NGHTTP2_REFUSED_STREAM' })); })); diff --git a/test/parallel/test-http2-respond-file-fd-invalid.js b/test/parallel/test-http2-respond-file-fd-invalid.js index 77a4d3df00d0d6..9e9cb276dc9678 100644 --- a/test/parallel/test-http2-respond-file-fd-invalid.js +++ b/test/parallel/test-http2-respond-file-fd-invalid.js @@ -13,7 +13,7 @@ const { const errorCheck = common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: `Stream closed with error code ${NGHTTP2_INTERNAL_ERROR}` + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' }, 2); const server = http2.createServer(); diff --git a/test/parallel/test-http2-respond-nghttperrors.js b/test/parallel/test-http2-respond-nghttperrors.js index 5ec953c5442360..ad9eee0d59fecc 100644 --- a/test/parallel/test-http2-respond-nghttperrors.js +++ b/test/parallel/test-http2-respond-nghttperrors.js @@ -80,7 +80,7 @@ function runTest(test) { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); currentError = test; diff --git a/test/parallel/test-http2-respond-with-fd-errors.js b/test/parallel/test-http2-respond-with-fd-errors.js index b7ff09225b6202..fae70a08e547c8 100644 --- a/test/parallel/test-http2-respond-with-fd-errors.js +++ b/test/parallel/test-http2-respond-with-fd-errors.js @@ -86,7 +86,7 @@ function runTest(test) { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 2' + message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR' })); currentError = test; diff --git a/test/parallel/test-http2-too-large-headers.js b/test/parallel/test-http2-too-large-headers.js index 7a7082736160f8..ad7a24f39b2fa2 100644 --- a/test/parallel/test-http2-too-large-headers.js +++ b/test/parallel/test-http2-too-large-headers.js @@ -20,7 +20,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 11' + message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM' })); req.on('close', common.mustCall((code) => { assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM); diff --git a/test/parallel/test-http2-too-many-headers.js b/test/parallel/test-http2-too-many-headers.js index f05511cff657e0..9b57060be695bb 100644 --- a/test/parallel/test-http2-too-many-headers.js +++ b/test/parallel/test-http2-too-many-headers.js @@ -23,7 +23,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 11' + message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM' })); req.on('close', common.mustCall((code) => { assert.strictEqual(code, NGHTTP2_ENHANCE_YOUR_CALM); diff --git a/test/sequential/test-http2-max-session-memory.js b/test/sequential/test-http2-max-session-memory.js index e16000d1261ab0..d6d3bf935db801 100644 --- a/test/sequential/test-http2-max-session-memory.js +++ b/test/sequential/test-http2-max-session-memory.js @@ -30,7 +30,7 @@ server.listen(0, common.mustCall(() => { req.on('error', common.expectsError({ code: 'ERR_HTTP2_STREAM_ERROR', type: Error, - message: 'Stream closed with error code 11' + message: 'Stream closed with error code NGHTTP2_ENHANCE_YOUR_CALM' })); req.on('close', common.mustCall(() => { server.close();