Skip to content

Commit

Permalink
errors: extract type detection & use in ERR_INVALID_RETURN_VALUE
Browse files Browse the repository at this point in the history
PR-URL: #43558
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
JakobJingleheimer authored and targos committed Jul 20, 2022
1 parent 3752c5b commit 0e17770
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 30 deletions.
55 changes: 30 additions & 25 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,31 @@ const genericNodeError = hideStackFrames(function genericNodeError(message, erro
return err;
});

/**
* Determine the specific type of a value for type-mismatch errors.
* @param {*} value
* @returns {string}
*/
function determineSpecificType(value) {
if (value == null) {
return '' + value;
}
if (typeof value === 'function' && value.name) {
return `function ${value.name}`;
}
if (typeof value === 'object') {
if (value.constructor?.name) {
return `an instance of ${value.constructor.name}`;
}
return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`;
}
let inspected = lazyInternalUtilInspect()
.inspect(value, { colors: false });
if (inspected.length > 28) { inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; }

return `type ${typeof value} (${inspected})`;
}

module.exports = {
AbortError,
aggregateTwoErrors,
Expand All @@ -853,6 +878,7 @@ module.exports = {
connResetException,
dnsException,
// This is exported only to facilitate testing.
determineSpecificType,
E,
errnoException,
exceptionWithHostPort,
Expand Down Expand Up @@ -1221,25 +1247,8 @@ E('ERR_INVALID_ARG_TYPE',
}
}

if (actual == null) {
msg += `. Received ${actual}`;
} else if (typeof actual === 'function' && actual.name) {
msg += `. Received function ${actual.name}`;
} else if (typeof actual === 'object') {
if (actual.constructor?.name) {
msg += `. Received an instance of ${actual.constructor.name}`;
} else {
const inspected = lazyInternalUtilInspect()
.inspect(actual, { depth: -1 });
msg += `. Received ${inspected}`;
}
} else {
let inspected = lazyInternalUtilInspect()
.inspect(actual, { colors: false });
if (inspected.length > 25)
inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`;
msg += `. Received type ${typeof actual} (${inspected})`;
}
msg += `. Received ${determineSpecificType(actual)}`;

return msg;
}, TypeError);
E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
Expand Down Expand Up @@ -1321,12 +1330,8 @@ E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) => {
` "${name}" function but got ${type}.`;
}, TypeError);
E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
let type;
if (value?.constructor?.name) {
type = `instance of ${value.constructor.name}`;
} else {
type = `type ${typeof value}`;
}
const type = determineSpecificType(value);

return `Expected ${input} to be returned from the "${name}"` +
` function but got ${type}.`;
}, TypeError, RangeError);
Expand Down
7 changes: 4 additions & 3 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -716,14 +716,15 @@ function invalidArgTypeHelper(input) {
return ` Received function ${input.name}`;
}
if (typeof input === 'object') {
if (input.constructor && input.constructor.name) {
if (input.constructor?.name) {
return ` Received an instance of ${input.constructor.name}`;
}
return ` Received ${inspect(input, { depth: -1 })}`;
}

let inspected = inspect(input, { colors: false });
if (inspected.length > 25)
inspected = `${inspected.slice(0, 25)}...`;
if (inspected.length > 28) { inspected = `${inspected.slice(inspected, 0, 25)}...`; }

return ` Received type ${typeof input} (${inspected})`;
}

Expand Down
150 changes: 150 additions & 0 deletions test/errors/test-error-value-type-detection.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// Flags: --expose-internals

import '../common/index.mjs';
import { strictEqual } from 'node:assert';
import errorsModule from 'internal/errors';


const { determineSpecificType } = errorsModule;

strictEqual(
determineSpecificType(1n),
'type bigint (1n)',
);

strictEqual(
determineSpecificType(false),
'type boolean (false)',
);

strictEqual(
determineSpecificType(2),
'type number (2)',
);

strictEqual(
determineSpecificType(NaN),
'type number (NaN)',
);

strictEqual(
determineSpecificType(Infinity),
'type number (Infinity)',
);

strictEqual(
determineSpecificType(Object.create(null)),
'[Object: null prototype] {}',
);

strictEqual(
determineSpecificType(''),
"type string ('')",
);

strictEqual(
determineSpecificType(Symbol('foo')),
'type symbol (Symbol(foo))',
);

strictEqual(
determineSpecificType(function foo() {}),
'function foo',
);

strictEqual(
determineSpecificType(null),
'null',
);

strictEqual(
determineSpecificType(undefined),
'undefined',
);

strictEqual(
determineSpecificType([]),
'an instance of Array',
);

strictEqual(
determineSpecificType(new Array()),
'an instance of Array',
);
strictEqual(
determineSpecificType(new BigInt64Array()),
'an instance of BigInt64Array',
);
strictEqual(
determineSpecificType(new BigUint64Array()),
'an instance of BigUint64Array',
);
strictEqual(
determineSpecificType(new Int8Array()),
'an instance of Int8Array',
);
strictEqual(
determineSpecificType(new Int16Array()),
'an instance of Int16Array',
);
strictEqual(
determineSpecificType(new Int32Array()),
'an instance of Int32Array',
);
strictEqual(
determineSpecificType(new Float32Array()),
'an instance of Float32Array',
);
strictEqual(
determineSpecificType(new Float64Array()),
'an instance of Float64Array',
);
strictEqual(
determineSpecificType(new Uint8Array()),
'an instance of Uint8Array',
);
strictEqual(
determineSpecificType(new Uint8ClampedArray()),
'an instance of Uint8ClampedArray',
);
strictEqual(
determineSpecificType(new Uint16Array()),
'an instance of Uint16Array',
);
strictEqual(
determineSpecificType(new Uint32Array()),
'an instance of Uint32Array',
);

strictEqual(
determineSpecificType(new Date()),
'an instance of Date',
);

strictEqual(
determineSpecificType(new Map()),
'an instance of Map',
);
strictEqual(
determineSpecificType(new WeakMap()),
'an instance of WeakMap',
);

strictEqual(
determineSpecificType({}),
'an instance of Object',
);

strictEqual(
determineSpecificType(Promise.resolve('foo')),
'an instance of Promise',
);

strictEqual(
determineSpecificType(new Set()),
'an instance of Set',
);
strictEqual(
determineSpecificType(new WeakSet()),
'an instance of WeakSet',
);
5 changes: 3 additions & 2 deletions test/parallel/test-assert-async.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ const invalidThenableFunc = () => {
promises.push(assert.rejects(promise, {
name: 'TypeError',
code: 'ERR_INVALID_RETURN_VALUE',
// FIXME(JakobJingleheimer): This should match on key words, like /Promise/ and /undefined/.
message: 'Expected instance of Promise to be returned ' +
'from the "promiseFn" function but got type undefined.'
'from the "promiseFn" function but got undefined.'
}));

promise = assert.rejects(Promise.resolve(), common.mustNotCall());
Expand Down Expand Up @@ -162,7 +163,7 @@ promises.push(assert.rejects(
let promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
promises.push(assert.rejects(promise, {
message: 'Expected instance of Promise to be returned ' +
'from the "promiseFn" function but got instance of Map.',
'from the "promiseFn" function but got an instance of Map.',
code: 'ERR_INVALID_RETURN_VALUE',
name: 'TypeError'
}));
Expand Down

0 comments on commit 0e17770

Please sign in to comment.