From 74be6feedfd0fdaa4f813f40672b43430beea3be Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Fri, 24 Jun 2022 12:09:55 +0200 Subject: [PATCH 1/7] errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` --- lib/internal/errors.js | 68 +++++--- .../test-error-value-type-detection.mjs | 149 ++++++++++++++++++ test/parallel/test-assert-async.js | 5 +- 3 files changed, 195 insertions(+), 27 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 7570315abe54d6..85c8bdf55a40af 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -858,6 +858,44 @@ 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) { + let type = ''; + + if (value == null) { + type += value; + } else if (typeof value === 'function' && value.name) { + type = `function ${value.name}`; + } else if (typeof value === 'object') { + if (value.constructor?.name) { + type = `an instance of ${value.constructor.name}`; + } else { + const inspected = lazyInternalUtilInspect() + .inspect(value, { depth: -1 }); + + if (StringPrototypeIncludes(inspected, '[Object: null prototype]')) { + type = 'an instance of Object'; + } else { + type = inspected; + } + } + } else { + let inspected = lazyInternalUtilInspect() + .inspect(value, { colors: false }); + if (inspected.length > 25) { + inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; + } + + type = `type ${typeof value} (${inspected})`; + } + + return type; +} + module.exports = { AbortError, aggregateTwoErrors, @@ -866,6 +904,7 @@ module.exports = { connResetException, dnsException, // This is exported only to facilitate testing. + determineSpecificType, E, errnoException, exceptionWithHostPort, @@ -1237,25 +1276,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') => { @@ -1335,12 +1357,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/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs new file mode 100644 index 00000000000000..2654b2ba864b7e --- /dev/null +++ b/test/errors/test-error-value-type-detection.mjs @@ -0,0 +1,149 @@ +// 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(''), + "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(0)), + 'an instance of Array', +); +strictEqual( + determineSpecificType(new BigInt64Array(0)), + 'an instance of BigInt64Array', +); +strictEqual( + determineSpecificType(new BigUint64Array(0)), + 'an instance of BigUint64Array', +); +strictEqual( + determineSpecificType(new Int8Array(0)), + 'an instance of Int8Array', +); +strictEqual( + determineSpecificType(new Int16Array(0)), + 'an instance of Int16Array', +); +strictEqual( + determineSpecificType(new Int32Array(0)), + 'an instance of Int32Array', +); +strictEqual( + determineSpecificType(new Float32Array(0)), + 'an instance of Float32Array', +); +strictEqual( + determineSpecificType(new Float64Array(0)), + 'an instance of Float64Array', +); +strictEqual( + determineSpecificType(new Uint8Array(0)), + 'an instance of Uint8Array', +); +strictEqual( + determineSpecificType(new Uint8ClampedArray(0)), + 'an instance of Uint8ClampedArray', +); +strictEqual( + determineSpecificType(new Uint16Array(0)), + 'an instance of Uint16Array', +); +strictEqual( + determineSpecificType(new Uint32Array(0)), + '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(Object.create(null)), + 'an instance of Object', +); +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..3927ec664124ed 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: This should use substring matching for 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' })); From 572c1621889be2c589358eb3fe69c638a0eda04b Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 26 Jun 2022 13:17:37 +0200 Subject: [PATCH 2/7] fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new Array(0)` et al → new Array(); tag self in FIXME --- .../test-error-value-type-detection.mjs | 24 +++++++++---------- test/parallel/test-assert-async.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs index 2654b2ba864b7e..f90b8f4a53dcf0 100644 --- a/test/errors/test-error-value-type-detection.mjs +++ b/test/errors/test-error-value-type-detection.mjs @@ -63,51 +63,51 @@ strictEqual( ); strictEqual( - determineSpecificType(new Array(0)), + determineSpecificType(new Array()), 'an instance of Array', ); strictEqual( - determineSpecificType(new BigInt64Array(0)), + determineSpecificType(new BigInt64Array()), 'an instance of BigInt64Array', ); strictEqual( - determineSpecificType(new BigUint64Array(0)), + determineSpecificType(new BigUint64Array()), 'an instance of BigUint64Array', ); strictEqual( - determineSpecificType(new Int8Array(0)), + determineSpecificType(new Int8Array()), 'an instance of Int8Array', ); strictEqual( - determineSpecificType(new Int16Array(0)), + determineSpecificType(new Int16Array()), 'an instance of Int16Array', ); strictEqual( - determineSpecificType(new Int32Array(0)), + determineSpecificType(new Int32Array()), 'an instance of Int32Array', ); strictEqual( - determineSpecificType(new Float32Array(0)), + determineSpecificType(new Float32Array()), 'an instance of Float32Array', ); strictEqual( - determineSpecificType(new Float64Array(0)), + determineSpecificType(new Float64Array()), 'an instance of Float64Array', ); strictEqual( - determineSpecificType(new Uint8Array(0)), + determineSpecificType(new Uint8Array()), 'an instance of Uint8Array', ); strictEqual( - determineSpecificType(new Uint8ClampedArray(0)), + determineSpecificType(new Uint8ClampedArray()), 'an instance of Uint8ClampedArray', ); strictEqual( - determineSpecificType(new Uint16Array(0)), + determineSpecificType(new Uint16Array()), 'an instance of Uint16Array', ); strictEqual( - determineSpecificType(new Uint32Array(0)), + determineSpecificType(new Uint32Array()), 'an instance of Uint32Array', ); diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js index 3927ec664124ed..7ea6d1723eaab8 100644 --- a/test/parallel/test-assert-async.js +++ b/test/parallel/test-assert-async.js @@ -103,7 +103,7 @@ const invalidThenableFunc = () => { promises.push(assert.rejects(promise, { name: 'TypeError', code: 'ERR_INVALID_RETURN_VALUE', - // FIXME: This should use substring matching for key words, like /Promise/ and /undefined/ + // 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 undefined.' })); From 4a4204e3bb6822c1a24737f58690616a0b133392 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 26 Jun 2022 13:53:29 +0200 Subject: [PATCH 3/7] fixup! fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit type object (…) --- lib/internal/errors.js | 15 ++------------- test/errors/test-error-value-type-detection.mjs | 9 +++++---- test/parallel/test-assert-async.js | 2 +- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 85c8bdf55a40af..418402e88dbe73 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -870,23 +870,12 @@ function determineSpecificType(value) { type += value; } else if (typeof value === 'function' && value.name) { type = `function ${value.name}`; - } else if (typeof value === 'object') { - if (value.constructor?.name) { + } else if (typeof value === 'object' && value.constructor?.name) { type = `an instance of ${value.constructor.name}`; - } else { - const inspected = lazyInternalUtilInspect() - .inspect(value, { depth: -1 }); - - if (StringPrototypeIncludes(inspected, '[Object: null prototype]')) { - type = 'an instance of Object'; - } else { - type = inspected; - } - } } else { let inspected = lazyInternalUtilInspect() .inspect(value, { colors: false }); - if (inspected.length > 25) { + if (inspected.length > 28) { inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; } diff --git a/test/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs index f90b8f4a53dcf0..2b2f6602d839e6 100644 --- a/test/errors/test-error-value-type-detection.mjs +++ b/test/errors/test-error-value-type-detection.mjs @@ -32,6 +32,11 @@ strictEqual( 'type number (Infinity)', ); +strictEqual( + determineSpecificType(Object.create(null)), + 'type object ([Object: null prototype] {})', +); + strictEqual( determineSpecificType(''), "type string ('')", @@ -125,10 +130,6 @@ strictEqual( 'an instance of WeakMap', ); -strictEqual( - determineSpecificType(Object.create(null)), - 'an instance of Object', -); strictEqual( determineSpecificType({}), 'an instance of Object', diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js index 7ea6d1723eaab8..bcf3d55622b863 100644 --- a/test/parallel/test-assert-async.js +++ b/test/parallel/test-assert-async.js @@ -103,7 +103,7 @@ 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/ + // 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 undefined.' })); From b757821630a1241640a131ae804d80613cf03c5f Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 26 Jun 2022 15:34:48 +0200 Subject: [PATCH 4/7] fixup! fixup! fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` de-lint --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 418402e88dbe73..51a182d9406335 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -871,7 +871,7 @@ function determineSpecificType(value) { } else if (typeof value === 'function' && value.name) { type = `function ${value.name}`; } else if (typeof value === 'object' && value.constructor?.name) { - type = `an instance of ${value.constructor.name}`; + type = `an instance of ${value.constructor.name}`; } else { let inspected = lazyInternalUtilInspect() .inspect(value, { colors: false }); From 4871b85cd55293d5789bc2a1ad6c6485343b586f Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Thu, 30 Jun 2022 22:14:41 +0200 Subject: [PATCH 5/7] fixup! fixup! fixup! fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` re-align test/common/invalidArgTypeHelper with implementation --- lib/internal/errors.js | 27 +++++++++++---------------- test/common/index.js | 12 +++++------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 51a182d9406335..a47aa7ad725f97 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -864,25 +864,20 @@ const genericNodeError = hideStackFrames(function genericNodeError(message, erro * @returns {string} */ function determineSpecificType(value) { - let type = ''; - if (value == null) { - type += value; - } else if (typeof value === 'function' && value.name) { - type = `function ${value.name}`; - } else if (typeof value === 'object' && value.constructor?.name) { - type = `an instance of ${value.constructor.name}`; - } else { - let inspected = lazyInternalUtilInspect() - .inspect(value, { colors: false }); - if (inspected.length > 28) { - inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; - } - - type = `type ${typeof value} (${inspected})`; + return '' + value; + } + if (typeof value === 'function' && value.name) { + return `function ${value.name}`; + } + if (typeof value === 'object' && value.constructor?.name) { + return `an instance of ${value.constructor.name}`; } + let inspected = lazyInternalUtilInspect() + .inspect(value, { colors: false }); + if (inspected.length > 28) { inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; } - return type; + return `type ${typeof value} (${inspected})`; } module.exports = { diff --git a/test/common/index.js b/test/common/index.js index 060deef1b8094c..920933b30570ca 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -736,15 +736,13 @@ function invalidArgTypeHelper(input) { if (typeof input === 'function' && input.name) { return ` Received function ${input.name}`; } - if (typeof input === 'object') { - if (input.constructor && input.constructor.name) { - return ` Received an instance of ${input.constructor.name}`; - } - return ` Received ${inspect(input, { depth: -1 })}`; + if (typeof input === 'object' && input.constructor?.name) { + return ` Received an instance of ${input.constructor.name}`; } + 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})`; } From 96dbd0e2e8a36259bc23c4347e033e5559ea0117 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Fri, 1 Jul 2022 14:57:07 +0200 Subject: [PATCH 6/7] fixup! fixup! fixup! fixup! fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` revert type determination output for objects without a constructor --- lib/internal/errors.js | 7 +++++-- test/common/index.js | 7 +++++-- test/errors/test-error-value-type-detection.mjs | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a47aa7ad725f97..ab958ad549801d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -870,8 +870,11 @@ function determineSpecificType(value) { if (typeof value === 'function' && value.name) { return `function ${value.name}`; } - if (typeof value === 'object' && value.constructor?.name) { - return `an instance of ${value.constructor.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 }); diff --git a/test/common/index.js b/test/common/index.js index 920933b30570ca..69bc6f5094e04a 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -736,8 +736,11 @@ function invalidArgTypeHelper(input) { if (typeof input === 'function' && input.name) { return ` Received function ${input.name}`; } - if (typeof input === 'object' && input.constructor?.name) { - return ` Received an instance of ${input.constructor.name}`; + if (typeof input === 'object') { + if (input?.constructor?.name) { + return ` Received an instance of ${input.constructor.name}`; + } + return ` Received ${inspect(input, { depth: -1 })}`; } let inspected = inspect(input, { colors: false }); diff --git a/test/errors/test-error-value-type-detection.mjs b/test/errors/test-error-value-type-detection.mjs index 2b2f6602d839e6..75e6493a9bbd09 100644 --- a/test/errors/test-error-value-type-detection.mjs +++ b/test/errors/test-error-value-type-detection.mjs @@ -34,7 +34,7 @@ strictEqual( strictEqual( determineSpecificType(Object.create(null)), - 'type object ([Object: null prototype] {})', + '[Object: null prototype] {}', ); strictEqual( From d2eded5cea4706d07e4186d23d27e72295edeab8 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Fri, 1 Jul 2022 15:19:39 +0200 Subject: [PATCH 7/7] fixup! fixup! fixup! fixup! fixup! fixup! errors: extract type detection & use in `ERR_INVALID_RETURN_VALUE` remove superfluous optional property access on constructor --- lib/internal/errors.js | 2 +- test/common/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index ab958ad549801d..87359a2d7c091b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -871,7 +871,7 @@ function determineSpecificType(value) { return `function ${value.name}`; } if (typeof value === 'object') { - if (value?.constructor?.name) { + if (value.constructor?.name) { return `an instance of ${value.constructor.name}`; } return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`; diff --git a/test/common/index.js b/test/common/index.js index 69bc6f5094e04a..efb882c21e4195 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -737,7 +737,7 @@ function invalidArgTypeHelper(input) { return ` Received function ${input.name}`; } if (typeof input === 'object') { - if (input?.constructor?.name) { + if (input.constructor?.name) { return ` Received an instance of ${input.constructor.name}`; } return ` Received ${inspect(input, { depth: -1 })}`;