From 7d7e34c15a471e02cd340f43abd30a90f5563334 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 3 Nov 2020 12:03:27 +0100 Subject: [PATCH] errors: refactor to use more primordials PR-URL: https://github.com/nodejs/node/pull/35944 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Ben Coe Reviewed-By: Rich Trott --- lib/internal/errors.js | 99 ++++++++++++------- .../source_map/prepare_stack_trace.js | 24 +++-- 2 files changed, 77 insertions(+), 46 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 447db182eb2ee3..45de8e81d6a204 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -11,31 +11,49 @@ // message may change, the code should not. const { + ArrayFrom, ArrayIsArray, + ArrayPrototypeIncludes, + ArrayPrototypeIndexOf, + ArrayPrototypeJoin, + ArrayPrototypeMap, + ArrayPrototypePop, + ArrayPrototypePush, + ArrayPrototypeSlice, + ArrayPrototypeSplice, + ArrayPrototypeUnshift, Error, ErrorCaptureStackTrace, ErrorPrototypeToString, JSONStringify, - Map, MathAbs, MathMax, + Number, NumberIsInteger, ObjectDefineProperty, ObjectKeys, RangeError, + ReflectApply, + RegExpPrototypeTest, + SafeMap, + SafeWeakMap, String, + StringPrototypeEndsWith, + StringPrototypeIncludes, + StringPrototypeSlice, + StringPrototypeSplit, StringPrototypeStartsWith, + StringPrototypeToLowerCase, Symbol, SymbolFor, SyntaxError, TypeError, URIError, - WeakMap, } = primordials; const isWindows = process.platform === 'win32'; -const messages = new Map(); +const messages = new SafeMap(); const codes = {}; const classRegExp = /^([A-Z][a-z0-9]*)+$/; @@ -54,7 +72,7 @@ const kTypes = [ ]; const MainContextError = Error; -const overrideStackTrace = new WeakMap(); +const overrideStackTrace = new SafeWeakMap(); const kNoOverride = Symbol('kNoOverride'); const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting @@ -366,8 +384,8 @@ function getMessage(key, args, self) { if (args.length === 0) return msg; - args.unshift(msg); - return lazyInternalUtilInspect().format.apply(null, args); + ArrayPrototypeUnshift(args, msg); + return ReflectApply(lazyInternalUtilInspect().format, null, args); } let uvBinding; @@ -640,9 +658,9 @@ function addNumericalSeparator(val) { let i = val.length; const start = val[0] === '-' ? 1 : 0; for (; i >= start + 4; i -= 3) { - res = `_${val.slice(i - 3, i)}${res}`; + res = `_${StringPrototypeSlice(val, i - 3, i)}${res}`; } - return `${val.slice(0, i)}${res}`; + return `${StringPrototypeSlice(val, 0, i)}${res}`; } // Used to enhance the stack that will be picked up by the inspector @@ -677,7 +695,8 @@ const fatalExceptionStackEnhancers = { // ANSI escape sequences is not reliable. if (process.platform === 'win32') { const info = internalBinding('os').getOSInformation(); - const ver = info[2].split('.').map((a) => +a); + const ver = ArrayPrototypeMap(StringPrototypeSplit(info[2], '.'), + Number); if (ver[0] !== 10 || ver[2] < 14393) { useColors = false; } @@ -981,11 +1000,11 @@ E('ERR_INVALID_ARG_TYPE', } let msg = 'The '; - if (name.endsWith(' argument')) { + if (StringPrototypeEndsWith(name, ' argument')) { // For cases like 'first argument' msg += `${name} `; } else { - const type = name.includes('.') ? 'property' : 'argument'; + const type = StringPrototypeIncludes(name, '.') ? 'property' : 'argument'; msg += `"${name}" ${type} `; } msg += 'must be '; @@ -997,31 +1016,31 @@ E('ERR_INVALID_ARG_TYPE', for (const value of expected) { assert(typeof value === 'string', 'All expected entries have to be of type string'); - if (kTypes.includes(value)) { - types.push(value.toLowerCase()); - } else if (classRegExp.test(value)) { - instances.push(value); + if (ArrayPrototypeIncludes(kTypes, value)) { + ArrayPrototypePush(types, StringPrototypeToLowerCase(value)); + } else if (RegExpPrototypeTest(classRegExp, value)) { + ArrayPrototypePush(instances, value); } else { assert(value !== 'object', 'The value "object" should be written as "Object"'); - other.push(value); + ArrayPrototypePush(other, value); } } // Special handle `object` in case other instances are allowed to outline // the differences between each other. if (instances.length > 0) { - const pos = types.indexOf('object'); + const pos = ArrayPrototypeIndexOf(types, 'object'); if (pos !== -1) { - types.splice(pos, 1); - instances.push('Object'); + ArrayPrototypeSplice(types, pos, 1); + ArrayPrototypePush(instances, 'Object'); } } if (types.length > 0) { if (types.length > 2) { - const last = types.pop(); - msg += `one of type ${types.join(', ')}, or ${last}`; + const last = ArrayPrototypePop(types); + msg += `one of type ${ArrayPrototypeJoin(types, ', ')}, or ${last}`; } else if (types.length === 2) { msg += `one of type ${types[0]} or ${types[1]}`; } else { @@ -1033,8 +1052,9 @@ E('ERR_INVALID_ARG_TYPE', if (instances.length > 0) { if (instances.length > 2) { - const last = instances.pop(); - msg += `an instance of ${instances.join(', ')}, or ${last}`; + const last = ArrayPrototypePop(instances); + msg += + `an instance of ${ArrayPrototypeJoin(instances, ', ')}, or ${last}`; } else { msg += `an instance of ${instances[0]}`; if (instances.length === 2) { @@ -1047,12 +1067,12 @@ E('ERR_INVALID_ARG_TYPE', if (other.length > 0) { if (other.length > 2) { - const last = other.pop(); - msg += `one of ${other.join(', ')}, or ${last}`; + const last = ArrayPrototypePop(other); + msg += `one of ${ArrayPrototypeJoin(other, ', ')}, or ${last}`; } else if (other.length === 2) { msg += `one of ${other[0]} or ${other[1]}`; } else { - if (other[0].toLowerCase() !== other[0]) + if (StringPrototypeToLowerCase(other[0]) !== other[0]) msg += 'an '; msg += `${other[0]}`; } @@ -1074,7 +1094,7 @@ E('ERR_INVALID_ARG_TYPE', let inspected = lazyInternalUtilInspect() .inspect(actual, { colors: false }); if (inspected.length > 25) - inspected = `${inspected.slice(0, 25)}...`; + inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; msg += `. Received type ${typeof actual} (${inspected})`; } return msg; @@ -1082,7 +1102,7 @@ E('ERR_INVALID_ARG_TYPE', E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => { let inspected = lazyInternalUtilInspect().inspect(value); if (inspected.length > 128) { - inspected = `${inspected.slice(0, 128)}...`; + inspected = `${StringPrototypeSlice(inspected, 0, 128)}...`; } return `The argument '${name}' ${reason}. Received ${inspected}`; }, TypeError, RangeError); @@ -1208,9 +1228,10 @@ E('ERR_MANIFEST_ASSERT_INTEGRITY', moduleURL }" does not match the expected integrity.`; if (realIntegrities.size) { - const sri = [...realIntegrities.entries()].map(([alg, dgs]) => { - return `${alg}-${dgs}`; - }).join(' '); + const sri = ArrayPrototypeJoin( + ArrayFrom(realIntegrities.entries(), ([alg, dgs]) => `${alg}-${dgs}`), + ' ' + ); msg += ` Integrities found are: ${sri}`; } else { msg += ' The resource was not found in the policy.'; @@ -1237,7 +1258,13 @@ E('ERR_MISSING_ARGS', assert(args.length > 0, 'At least one arg needs to be specified'); let msg = 'The '; const len = args.length; - args = args.map((a) => `"${a}"`); + const wrap = (a) => `"${a}"`; + args = ArrayPrototypeMap( + args, + (a) => (ArrayIsArray(a) ? + ArrayPrototypeJoin(ArrayPrototypeMap(a, wrap), ' or ') : + wrap(a)) + ); switch (len) { case 1: msg += `${args[0]} argument`; @@ -1246,7 +1273,7 @@ E('ERR_MISSING_ARGS', msg += `${args[0]} and ${args[1]} arguments`; break; default: - msg += args.slice(0, len - 1).join(', '); + msg += ArrayPrototypeJoin(ArrayPrototypeSlice(args, 0, len - 1), ', '); msg += `, and ${args[len - 1]} arguments`; break; } @@ -1449,7 +1476,7 @@ E('ERR_VM_MODULE_STATUS', 'Module status %s', Error); E('ERR_WASI_ALREADY_STARTED', 'WASI instance has already started', Error); E('ERR_WORKER_INIT_FAILED', 'Worker initialization failure: %s', Error); E('ERR_WORKER_INVALID_EXEC_ARGV', (errors, msg = 'invalid execArgv flags') => - `Initiated Worker with ${msg}: ${errors.join(', ')}`, + `Initiated Worker with ${msg}: ${ArrayPrototypeJoin(errors, ', ')}`, Error); E('ERR_WORKER_NOT_RUNNING', 'Worker instance not running', Error); E('ERR_WORKER_OUT_OF_MEMORY', @@ -1457,10 +1484,10 @@ E('ERR_WORKER_OUT_OF_MEMORY', E('ERR_WORKER_PATH', (filename) => 'The worker script or module filename must be an absolute path or a ' + 'relative path starting with \'./\' or \'../\'.' + - (filename.startsWith('file://') ? + (StringPrototypeStartsWith(filename, 'file://') ? ' Wrap file:// URLs with `new URL`.' : '' ) + - (filename.startsWith('data:text/javascript') ? + (StringPrototypeStartsWith(filename, 'data:text/javascript') ? ' Wrap data: URLs with `new URL`.' : '' ) + ` Received "${filename}"`, diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index e9c3536c963504..7053a890a8e5a1 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -2,7 +2,12 @@ const { ArrayPrototypeIndexOf, - Error, + ArrayPrototypeJoin, + ArrayPrototypeMap, + ErrorPrototypeToString, + StringPrototypeRepeat, + StringPrototypeSlice, + StringPrototypeSplit, StringPrototypeStartsWith, } = primordials; @@ -21,7 +26,6 @@ const { fileURLToPath } = require('internal/url'); // Create a prettified stacktrace, inserting context from source maps // if possible. -const ErrorToString = Error.prototype.toString; // Capture original toString. const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting // without interfering with userland code. @@ -36,7 +40,7 @@ const prepareStackTrace = (globalThis, error, trace) => { maybeOverridePrepareStackTrace(globalThis, error, trace); if (globalOverride !== kNoOverride) return globalOverride; - const errorString = ErrorToString.call(error); + const errorString = ErrorPrototypeToString(error); if (trace.length === 0) { return errorString; @@ -45,7 +49,7 @@ const prepareStackTrace = (globalThis, error, trace) => { let errorSource = ''; let firstLine; let firstColumn; - const preparedTrace = trace.map((t, i) => { + const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => { if (i === 0) { firstLine = t.getLineNumber(); firstColumn = t.getColumnNumber(); @@ -88,8 +92,8 @@ const prepareStackTrace = (globalThis, error, trace) => { debug(err.stack); } return str; - }); - return `${errorSource}${errorString}\n at ${preparedTrace.join('')}`; + }), ''); + return `${errorSource}${errorString}\n at ${preparedTrace}`; }; // Places a snippet of code from where the exception was originally thrown @@ -118,18 +122,18 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) { } } - const lines = source.split(/\r?\n/, firstLine); + const lines = StringPrototypeSplit(source, /\r?\n/, firstLine); const line = lines[firstLine - 1]; if (!line) return exceptionLine; // Display ^ in appropriate position, regardless of whether tabs or // spaces are used: let prefix = ''; - for (const character of line.slice(0, firstColumn)) { + for (const character of StringPrototypeSlice(line, 0, firstColumn)) { prefix += (character === '\t') ? '\t' : - ' '.repeat(getStringWidth(character)); + StringPrototypeRepeat(' ', getStringWidth(character)); } - prefix = prefix.slice(0, -1); // The last character is the '^'. + prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'. exceptionLine = `${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`;