From 39f4158bc38ff2925b86563082c8d26ab3d20b17 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 ba948c61948..79a12dd4350 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 3fbbbe1a2fe..244e8666a9b 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 7b00bed3168..d283fda1435 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 4893cdc96b0..39e9dd625a5 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -809,7 +809,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; } @@ -1216,12 +1215,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 0a3faa23557..81b28e2ed47 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 08ae050b959..17af7566685 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 ecf7258e8bb..a5f012ac330 100644 --- a/lib/net.js +++ b/lib/net.js @@ -968,19 +968,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 9d2b9add66a..024a81289a1 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -258,11 +258,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 007be66f451..5661aee841d 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' + })); }