Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: extract type detection & use in ERR_INVALID_RETURN_VALUE #43558

55 changes: 30 additions & 25 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (value?.constructor?.name) {
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 @@ -866,6 +891,7 @@ module.exports = {
connResetException,
dnsException,
// This is exported only to facilitate testing.
determineSpecificType,
E,
errnoException,
exceptionWithHostPort,
Expand Down Expand Up @@ -1237,25 +1263,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 @@ -1335,12 +1344,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 @@ -737,14 +737,15 @@ function invalidArgTypeHelper(input) {
return ` Received function ${input.name}`;
}
if (typeof input === 'object') {
if (input.constructor && input.constructor.name) {
if (input?.constructor?.name) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (input?.constructor?.name) {
if (input.constructor?.name) {

We know for certain that input is an object of some kind (null is already checked above).

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