From bff5d301bf8e9c49b0ba04df5c00c55707d489e1 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 18 Mar 2019 15:41:19 +0100 Subject: [PATCH] lib: move extra properties into error creation This encapsulates the Node.js errors more by adding extra properties to an error inside of the function to create the error message instead of adding the properties at the call site. That simplifies the usage of our errors and makes sure the expected properties are always set. PR-URL: https://github.com/nodejs/node/pull/26752 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/_stream_readable.js | 4 +- lib/internal/encoding.js | 4 +- lib/internal/errors.js | 69 +++++++++++++++++++----- lib/internal/http2/core.js | 8 +-- lib/internal/http2/util.js | 6 +-- lib/internal/url.js | 4 +- lib/net.js | 17 +++--- lib/tls.js | 6 +-- test/parallel/test-net-options-lookup.js | 7 ++- 9 files changed, 78 insertions(+), 47 deletions(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 1bb86ab99d6932..1c1bce6cf2c376 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -309,15 +309,13 @@ function addChunk(stream, state, chunk, addToFront) { } function chunkInvalid(state, chunk) { - var er; if (!Stream._isUint8Array(chunk) && typeof chunk !== 'string' && chunk !== undefined && !state.objectMode) { - er = new ERR_INVALID_ARG_TYPE( + return new ERR_INVALID_ARG_TYPE( 'chunk', ['string', 'Buffer', 'Uint8Array'], chunk); } - return er; } diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index 0084c19b904a82..92a50952516536 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -394,9 +394,7 @@ function makeTextDecoderICU() { const ret = _decode(this[kHandle], input, flags); if (typeof ret === 'number') { - const err = new ERR_ENCODING_INVALID_ENCODED_DATA(this.encoding); - err.errno = ret; - throw err; + throw new ERR_ENCODING_INVALID_ENCODED_DATA(this.encoding, ret); } return ret.toString('ucs2'); } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a76153297ecd69..a80cf015242bfd 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -47,7 +47,8 @@ function lazyBuffer() { // and may have .path and .dest. class SystemError extends Error { constructor(key, context) { - const prefix = getMessage(key, []); + super(); + const prefix = getMessage(key, [], this); let message = `${prefix}: ${context.syscall} returned ` + `${context.code} (${context.message})`; @@ -56,7 +57,12 @@ class SystemError extends Error { if (context.dest !== undefined) message += ` => ${context.dest}`; - super(message); + Object.defineProperty(this, 'message', { + value: message, + enumerable: false, + writable: true, + configurable: true + }); Object.defineProperty(this, kInfo, { configurable: false, enumerable: false, @@ -150,7 +156,14 @@ let useOriginalName = false; function makeNodeErrorWithCode(Base, key) { return class NodeError extends Base { constructor(...args) { - super(getMessage(key, args)); + super(); + const message = getMessage(key, args, this); + Object.defineProperty(this, 'message', { + value: message, + enumerable: false, + writable: true, + configurable: true + }); } get name() { @@ -204,7 +217,7 @@ function E(sym, val, def, ...otherClasses) { codes[sym] = def; } -function getMessage(key, args) { +function getMessage(key, args, self) { const msg = messages.get(key); if (util === undefined) util = require('util'); @@ -216,7 +229,7 @@ function getMessage(key, args) { `Code: ${key}; The provided arguments length (${args.length}) does not ` + `match the required ones (${msg.length}).` ); - return msg.apply(null, args); + return msg.apply(self, args); } const expectedLength = (msg.match(/%[dfijoOs]/g) || []).length; @@ -608,8 +621,10 @@ E('ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE', 'The `domain` module is in use, which is mutually exclusive with calling ' + 'process.setUncaughtExceptionCaptureCallback()', Error); -E('ERR_ENCODING_INVALID_ENCODED_DATA', - 'The encoded data was not valid for encoding %s', TypeError); +E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) { + this.errno = ret; + return `The encoded data was not valid for encoding ${encoding}`; +}, TypeError); E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported', RangeError); E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value', Error); @@ -652,7 +667,16 @@ E('ERR_HTTP2_INVALID_PSEUDOHEADER', '"%s" is an invalid pseudoheader or is used incorrectly', TypeError); E('ERR_HTTP2_INVALID_SESSION', 'The session has been destroyed', Error); E('ERR_HTTP2_INVALID_SETTING_VALUE', - 'Invalid value for setting "%s": %s', TypeError, RangeError); + // Using default arguments here is important so the arguments are not counted + // towards `Function#length`. + function(name, actual, min = undefined, max = undefined) { + this.actual = actual; + if (min !== undefined) { + this.min = min; + this.max = max; + } + return `Invalid value for setting "${name}": ${actual}`; + }, TypeError, RangeError); E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed', Error); E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK', 'Maximum number of pending settings acknowledgements', Error); @@ -685,7 +709,15 @@ E('ERR_HTTP2_SOCKET_UNBOUND', E('ERR_HTTP2_STATUS_101', 'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2', Error); E('ERR_HTTP2_STATUS_INVALID', 'Invalid status code: %s', RangeError); -E('ERR_HTTP2_STREAM_CANCEL', 'The pending stream has been canceled', Error); +E('ERR_HTTP2_STREAM_CANCEL', function(error) { + let msg = 'The pending stream has been canceled'; + if (error) { + this.cause = error; + if (typeof error.message === 'string') + msg += ` (caused by: ${error.message})`; + } + return msg; +}, Error); E('ERR_HTTP2_STREAM_ERROR', 'Stream closed with error code %s', Error); E('ERR_HTTP2_STREAM_SELF_DEPENDENCY', 'A stream cannot depend on itself', Error); @@ -709,7 +741,11 @@ E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error); E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error); E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error); E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error); -E('ERR_INVALID_ADDRESS_FAMILY', 'Invalid address family: %s', RangeError); +E('ERR_INVALID_ADDRESS_FAMILY', function(addressType, host, port) { + this.host = host; + this.port = port; + return `Invalid address family: ${addressType} ${host}:${port}`; +}, RangeError); E('ERR_INVALID_ARG_TYPE', (name, expected, actual) => { assert(typeof name === 'string', "'name' must be a string"); @@ -812,7 +848,10 @@ E('ERR_INVALID_SYNC_FORK_INPUT', E('ERR_INVALID_THIS', 'Value of "this" must be of type %s', TypeError); E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple', TypeError); E('ERR_INVALID_URI', 'URI malformed', URIError); -E('ERR_INVALID_URL', 'Invalid URL: %s', TypeError); +E('ERR_INVALID_URL', function(input) { + this.input = input; + return `Invalid URL: ${input}`; +}, TypeError); E('ERR_INVALID_URL_SCHEME', (expected) => `The URL must be ${oneOf(expected, 'scheme')}`, TypeError); E('ERR_IPC_CHANNEL_CLOSED', 'Channel closed', Error); @@ -926,8 +965,12 @@ E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode', Error); E('ERR_STREAM_WRITE_AFTER_END', 'write after end', Error); E('ERR_SYNTHETIC', 'JavaScript Callstack', Error); E('ERR_SYSTEM_ERROR', 'A system error occurred', SystemError); -E('ERR_TLS_CERT_ALTNAME_INVALID', - 'Hostname/IP does not match certificate\'s altnames: %s', Error); +E('ERR_TLS_CERT_ALTNAME_INVALID', function(reason, host, cert) { + this.reason = reason; + this.host = host; + this.cert = cert; + return `Hostname/IP does not match certificate's altnames: ${reason}`; +}, Error); E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048', Error); E('ERR_TLS_HANDSHAKE_TIMEOUT', 'TLS handshake timeout', Error); E('ERR_TLS_INVALID_PROTOCOL_VERSION', diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 0e795d5aa1e397..b2a453d2e6057e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -812,7 +812,6 @@ function validateSettings(settings) { typeof settings.enablePush !== 'boolean') { const err = new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush', settings.enablePush); - err.actual = settings.enablePush; Error.captureStackTrace(err, 'validateSettings'); throw err; } @@ -1219,12 +1218,7 @@ class Http2Session extends EventEmitter { this.removeAllListeners('timeout'); // Destroy any pending and open streams - const cancel = new ERR_HTTP2_STREAM_CANCEL(); - if (error) { - cancel.cause = error; - if (typeof error.message === 'string') - cancel.message += ` (caused by: ${error.message})`; - } + const cancel = new ERR_HTTP2_STREAM_CANCEL(error); state.pendingStreams.forEach((stream) => stream.destroy(cancel)); state.streams.forEach((stream) => stream.destroy(error)); diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 0a3faa23557a96..81b28e2ed475fd 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -515,10 +515,8 @@ function assertIsObject(value, name, types) { function assertWithinRange(name, value, min = 0, max = Infinity) { if (value !== undefined && (typeof value !== 'number' || value < min || value > max)) { - const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(name, value); - err.min = min; - err.max = max; - err.actual = value; + const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError( + name, value, min, max); Error.captureStackTrace(err, assertWithinRange); throw err; } diff --git a/lib/internal/url.js b/lib/internal/url.js index c6d9ef8864a8e6..c2e2519b6e2334 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -238,9 +238,7 @@ function onParseComplete(flags, protocol, username, password, } function onParseError(flags, input) { - const error = new ERR_INVALID_URL(input); - error.input = input; - throw error; + throw new ERR_INVALID_URL(input); } function onParseProtocolComplete(flags, protocol, username, password, diff --git a/lib/net.js b/lib/net.js index 3e0a579c0c270c..d363878a929404 100644 --- a/lib/net.js +++ b/lib/net.js @@ -989,19 +989,20 @@ function lookupAndConnect(self, options) { if (!self.connecting) return; if (err) { - // net.createConnection() creates a net.Socket object and - // immediately calls net.Socket.connect() on it (that's us). - // There are no event listeners registered yet so defer the - // error event to the next tick. + // net.createConnection() creates a net.Socket object and immediately + // calls net.Socket.connect() on it (that's us). There are no event + // listeners registered yet so defer the error event to the next tick. + // TODO(BridgeAR): The error could either originate from user code or + // by the C++ layer. The port is never the cause for the error as it is + // not used in the lookup. We should probably just remove this. err.host = options.host; err.port = options.port; err.message = err.message + ' ' + options.host + ':' + options.port; process.nextTick(connectErrorNT, self, err); } else if (addressType !== 4 && addressType !== 6) { - err = new ERR_INVALID_ADDRESS_FAMILY(addressType); - err.host = options.host; - err.port = options.port; - err.message = err.message + ' ' + options.host + ':' + options.port; + err = new ERR_INVALID_ADDRESS_FAMILY(addressType, + options.host, + options.port); process.nextTick(connectErrorNT, self, err); } else { self._unrefTimer(); diff --git a/lib/tls.js b/lib/tls.js index 65eb8ae9834deb..b1bb591760f10c 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -242,11 +242,7 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { } if (!valid) { - const err = new ERR_TLS_CERT_ALTNAME_INVALID(reason); - err.reason = reason; - err.host = hostname; - err.cert = cert; - return err; + return new ERR_TLS_CERT_ALTNAME_INVALID(reason, hostname, cert); } }; diff --git a/test/parallel/test-net-options-lookup.js b/test/parallel/test-net-options-lookup.js index 007be66f4516bb..5661aee841d197 100644 --- a/test/parallel/test-net-options-lookup.js +++ b/test/parallel/test-net-options-lookup.js @@ -38,5 +38,10 @@ function connectDoesNotThrow(input) { cb(null, '127.0.0.1', 100); }); - s.on('error', common.expectsError({ code: 'ERR_INVALID_ADDRESS_FAMILY' })); + s.on('error', common.expectsError({ + code: 'ERR_INVALID_ADDRESS_FAMILY', + host: 'localhost', + port: 0, + message: 'Invalid address family: 100 localhost:0' + })); }