Skip to content

Commit

Permalink
http2: refer to stream errors by name
Browse files Browse the repository at this point in the history
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 <[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]>
  • Loading branch information
addaleax authored and rvagg committed Aug 16, 2018
1 parent 21cdb73 commit a29cd25
Show file tree
Hide file tree
Showing 19 changed files with 58 additions and 43 deletions.
5 changes: 3 additions & 2 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
59 changes: 36 additions & 23 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2961,29 +2961,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);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-client-unescaped-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()));

Expand All @@ -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()));

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-info-headers-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-max-concurrent-streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}));
}
}));
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-misbehaving-flow-control.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-misused-pseudoheaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-multi-content-length.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}));
}
}));
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}));
}));
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-file-fd-invalid.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-nghttperrors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-respond-with-fd-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-too-large-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http2-too-many-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/sequential/test-http2-max-session-memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a29cd25

Please sign in to comment.