Skip to content

Commit

Permalink
errors: refactor to use more primordials
Browse files Browse the repository at this point in the history 
PR-URL: #35944
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
@aduh95 @danielleadams
aduh95 authored and danielleadams committed Nov 9, 2020
1 parent b700900 commit d83e253
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 50 deletions.
105 changes: 65 additions & 40 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]*)+$/;
Expand All @@ -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
Expand Down Expand Up @@ -370,8 +388,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;
Expand Down Expand Up @@ -644,9 +662,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
Expand Down Expand Up @@ -681,7 +699,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;
}
Expand Down Expand Up @@ -975,11 +994,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 ';
Expand All @@ -991,31 +1010,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 {
Expand All @@ -1027,8 +1046,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) {
Expand All @@ -1041,12 +1061,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]}`;
}
Expand All @@ -1068,17 +1088,17 @@ 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;
}, TypeError);
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)}...`;
}
const type = name.includes('.') ? 'property' : 'argument';
const type = StringPrototypeIncludes(name, '.') ? 'property' : 'argument';
return `The ${type} '${name}' ${reason}. Received ${inspected}`;
}, TypeError, RangeError);
E('ERR_INVALID_ASYNC_ID', 'Invalid %s value: %s', RangeError);
Expand Down Expand Up @@ -1195,9 +1215,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.';
Expand Down Expand Up @@ -1225,8 +1246,11 @@ E('ERR_MISSING_ARGS',
let msg = 'The ';
const len = args.length;
const wrap = (a) => `"${a}"`;
args = args.map(
(a) => (ArrayIsArray(a) ? a.map(wrap).join(' or ') : wrap(a))
args = ArrayPrototypeMap(
args,
(a) => (ArrayIsArray(a) ?
ArrayPrototypeJoin(ArrayPrototypeMap(a, wrap), ' or ') :
wrap(a))
);
switch (len) {
case 1:
Expand All @@ -1236,7 +1260,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;
}
Expand Down Expand Up @@ -1299,9 +1323,10 @@ E('ERR_QUIC_INVALID_TLS_SESSION_TICKET',
'Invalid TLS session ticket', Error);
E('ERR_QUIC_VERSION_NEGOTIATION',
(version, requestedVersions, supportedVersions) => {
const requestedVersionsString = ArrayPrototypeJoin(requestedVersions, ', ');
return 'QUIC session received version negotiation from server. ' +
`Version: ${version}. Requested: ${requestedVersions.join(', ')} ` +
`Supported: ${supportedVersions.join(', ')}`;
`Version: ${version}. Requested: ${requestedVersionsString} ` +
`Supported: ${ArrayPrototypeJoin(supportedVersions, ', ')}`;
},
Error);
E('ERR_REQUIRE_ESM',
Expand Down Expand Up @@ -1447,18 +1472,18 @@ 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',
'Worker terminated due to reaching memory limit: %s', Error);
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}"`,
Expand Down
24 changes: 14 additions & 10 deletions lib/internal/source_map/prepare_stack_trace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

const {
ArrayPrototypeIndexOf,
Error,
ArrayPrototypeJoin,
ArrayPrototypeMap,
ErrorPrototypeToString,
StringPrototypeRepeat,
StringPrototypeSlice,
StringPrototypeSplit,
StringPrototypeStartsWith,
} = primordials;

Expand All @@ -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.
Expand All @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`;
Expand Down

0 comments on commit d83e253

Please sign in to comment.