From 9f75f26ad9c15ed0bb3bc274e44621e38a4894a8 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Fri, 1 Jul 2022 20:08:08 +0200 Subject: [PATCH] errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` PR-URL: https://github.com/nodejs/node/pull/43558 Reviewed-By: Ruben Bridgewater Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell --- lib/internal/errors.js | 55 ++++--- test/common/index.js | 7 +- .../test-error-value-type-detection.mjs | 150 ++++++++++++++++++ test/parallel/test-assert-async.js | 5 +- 4 files changed, 187 insertions(+), 30 deletions(-) create mode 100644 test/errors/test-error-value-type-detection.mjs diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 6184ca245127b1..fc341c4e2b17ac 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -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, @@ -853,6 +878,7 @@ module.exports = { connResetException, dnsException, // This is exported only to facilitate testing. + determineSpecificType, E, errnoException, exceptionWithHostPort, @@ -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') => { @@ -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); diff --git a/test/common/index.js b/test/common/index.js index 4921fb5fa8ec3d..2f056c96e8eb85 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -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})`; } diff --git a/test/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs new file mode 100644 index 00000000000000..75e6493a9bbd09 --- /dev/null +++ b/test/errors/test-error-value-type-detection.mjs @@ -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', +); diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js index 1a192ae3f7da64..bcf3d55622b863 100644 --- a/test/parallel/test-assert-async.js +++ b/test/parallel/test-assert-async.js @@ -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()); @@ -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' }));