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. Backport-PR-URL: #19579 PR-URL: #18966 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Mar 30, 2018 · 441175c · 441175c
1 parent 4caf536
commit 441175c
Show file tree

Hide file tree

Showing 20 changed files with 61 additions and 46 deletions.
diff --git 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;
@@ -1841,7 +1841,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
@@ -2857,29 +2857,39 @@ void Initialize(Local<Object> target,
               session->GetFunction()).FromJust();
 
   Local<Object> 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<Array> 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<int>(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);
@@ -2944,6 +2954,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
@@ -47,7 +47,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
@@ -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
@@ -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
@@ -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 2'
+      message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
     }));
     req.on('close', common.mustCall(() => countdown.dec()));
 
@@ -73,7 +73,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
@@ -72,7 +72,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
@@ -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
@@ -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
@@ -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
@@ -41,7 +41,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
@@ -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
@@ -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
@@ -15,7 +15,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
@@ -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
@@ -88,7 +88,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-server-rst-stream.js b/test/parallel/test-http2-server-rst-stream.js
@@ -18,10 +18,10 @@ const {
 const tests = [
   [NGHTTP2_NO_ERROR, false],
   [NGHTTP2_NO_ERROR, false],
-  [NGHTTP2_PROTOCOL_ERROR, true, 1],
+  [NGHTTP2_PROTOCOL_ERROR, true, 'NGHTTP2_PROTOCOL_ERROR'],
   [NGHTTP2_CANCEL, false],
-  [NGHTTP2_REFUSED_STREAM, true, 7],
-  [NGHTTP2_INTERNAL_ERROR, true, 2]
+  [NGHTTP2_REFUSED_STREAM, true, 'NGHTTP2_REFUSED_STREAM'],
+  [NGHTTP2_INTERNAL_ERROR, true, 'NGHTTP2_INTERNAL_ERROR']
 ];
 
 const server = http2.createServer();

diff --git 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
@@ -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
@@ -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();