Skip to content

Commit 79ea19e

Browse files
JakobJingleheimertargos
JakobJingleheimer
authored and
targos
committed
errors: extract type detection & use in ERR_INVALID_RETURN_VALUE
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>
1 parent 6ede1c2 commit 79ea19e

File tree

4 files changed

+187
-30
lines changed

4 files changed

+187
-30
lines changed

‎lib/internal/errors.js

+30-25
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,31 @@ const genericNodeError = hideStackFrames(function genericNodeError(message, erro
858858
return err;
859859
});
860860

861+
/**
862+
* Determine the specific type of a value for type-mismatch errors.
863+
* @param {*} value
864+
* @returns {string}
865+
*/
866+
function determineSpecificType(value) {
867+
if (value == null) {
868+
return '' + value;
869+
}
870+
if (typeof value === 'function' && value.name) {
871+
return `function ${value.name}`;
872+
}
873+
if (typeof value === 'object') {
874+
if (value.constructor?.name) {
875+
return `an instance of ${value.constructor.name}`;
876+
}
877+
return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`;
878+
}
879+
let inspected = lazyInternalUtilInspect()
880+
.inspect(value, { colors: false });
881+
if (inspected.length > 28) { inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`; }
882+
883+
return `type ${typeof value} (${inspected})`;
884+
}
885+
861886
module.exports = {
862887
AbortError,
863888
aggregateTwoErrors,
@@ -866,6 +891,7 @@ module.exports = {
866891
connResetException,
867892
dnsException,
868893
// This is exported only to facilitate testing.
894+
determineSpecificType,
869895
E,
870896
errnoException,
871897
exceptionWithHostPort,
@@ -1237,25 +1263,8 @@ E('ERR_INVALID_ARG_TYPE',
12371263
}
12381264
}
12391265

1240-
if (actual == null) {
1241-
msg += `. Received ${actual}`;
1242-
} else if (typeof actual === 'function' && actual.name) {
1243-
msg += `. Received function ${actual.name}`;
1244-
} else if (typeof actual === 'object') {
1245-
if (actual.constructor?.name) {
1246-
msg += `. Received an instance of ${actual.constructor.name}`;
1247-
} else {
1248-
const inspected = lazyInternalUtilInspect()
1249-
.inspect(actual, { depth: -1 });
1250-
msg += `. Received ${inspected}`;
1251-
}
1252-
} else {
1253-
let inspected = lazyInternalUtilInspect()
1254-
.inspect(actual, { colors: false });
1255-
if (inspected.length > 25)
1256-
inspected = `${StringPrototypeSlice(inspected, 0, 25)}...`;
1257-
msg += `. Received type ${typeof actual} (${inspected})`;
1258-
}
1266+
msg += `. Received ${determineSpecificType(actual)}`;
1267+
12591268
return msg;
12601269
}, TypeError);
12611270
E('ERR_INVALID_ARG_VALUE', (name, value, reason = 'is invalid') => {
@@ -1335,12 +1344,8 @@ E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) => {
13351344
` "${name}" function but got ${type}.`;
13361345
}, TypeError);
13371346
E('ERR_INVALID_RETURN_VALUE', (input, name, value) => {
1338-
let type;
1339-
if (value?.constructor?.name) {
1340-
type = `instance of ${value.constructor.name}`;
1341-
} else {
1342-
type = `type ${typeof value}`;
1343-
}
1347+
const type = determineSpecificType(value);
1348+
13441349
return `Expected ${input} to be returned from the "${name}"` +
13451350
` function but got ${type}.`;
13461351
}, TypeError, RangeError);

‎test/common/index.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -737,14 +737,15 @@ function invalidArgTypeHelper(input) {
737737
return ` Received function ${input.name}`;
738738
}
739739
if (typeof input === 'object') {
740-
if (input.constructor && input.constructor.name) {
740+
if (input.constructor?.name) {
741741
return ` Received an instance of ${input.constructor.name}`;
742742
}
743743
return ` Received ${inspect(input, { depth: -1 })}`;
744744
}
745+
745746
let inspected = inspect(input, { colors: false });
746-
if (inspected.length > 25)
747-
inspected = `${inspected.slice(0, 25)}...`;
747+
if (inspected.length > 28) { inspected = `${inspected.slice(inspected, 0, 25)}...`; }
748+
748749
return ` Received type ${typeof input} (${inspected})`;
749750
}
750751

‎test/errors/test-error-value-type-detection.mjs

+150
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// Flags: --expose-internals
2+
3+
import '../common/index.mjs';
4+
import { strictEqual } from 'node:assert';
5+
import errorsModule from 'internal/errors';
6+
7+
8+
const { determineSpecificType } = errorsModule;
9+
10+
strictEqual(
11+
determineSpecificType(1n),
12+
'type bigint (1n)',
13+
);
14+
15+
strictEqual(
16+
determineSpecificType(false),
17+
'type boolean (false)',
18+
);
19+
20+
strictEqual(
21+
determineSpecificType(2),
22+
'type number (2)',
23+
);
24+
25+
strictEqual(
26+
determineSpecificType(NaN),
27+
'type number (NaN)',
28+
);
29+
30+
strictEqual(
31+
determineSpecificType(Infinity),
32+
'type number (Infinity)',
33+
);
34+
35+
strictEqual(
36+
determineSpecificType(Object.create(null)),
37+
'[Object: null prototype] {}',
38+
);
39+
40+
strictEqual(
41+
determineSpecificType(''),
42+
"type string ('')",
43+
);
44+
45+
strictEqual(
46+
determineSpecificType(Symbol('foo')),
47+
'type symbol (Symbol(foo))',
48+
);
49+
50+
strictEqual(
51+
determineSpecificType(function foo() {}),
52+
'function foo',
53+
);
54+
55+
strictEqual(
56+
determineSpecificType(null),
57+
'null',
58+
);
59+
60+
strictEqual(
61+
determineSpecificType(undefined),
62+
'undefined',
63+
);
64+
65+
strictEqual(
66+
determineSpecificType([]),
67+
'an instance of Array',
68+
);
69+
70+
strictEqual(
71+
determineSpecificType(new Array()),
72+
'an instance of Array',
73+
);
74+
strictEqual(
75+
determineSpecificType(new BigInt64Array()),
76+
'an instance of BigInt64Array',
77+
);
78+
strictEqual(
79+
determineSpecificType(new BigUint64Array()),
80+
'an instance of BigUint64Array',
81+
);
82+
strictEqual(
83+
determineSpecificType(new Int8Array()),
84+
'an instance of Int8Array',
85+
);
86+
strictEqual(
87+
determineSpecificType(new Int16Array()),
88+
'an instance of Int16Array',
89+
);
90+
strictEqual(
91+
determineSpecificType(new Int32Array()),
92+
'an instance of Int32Array',
93+
);
94+
strictEqual(
95+
determineSpecificType(new Float32Array()),
96+
'an instance of Float32Array',
97+
);
98+
strictEqual(
99+
determineSpecificType(new Float64Array()),
100+
'an instance of Float64Array',
101+
);
102+
strictEqual(
103+
determineSpecificType(new Uint8Array()),
104+
'an instance of Uint8Array',
105+
);
106+
strictEqual(
107+
determineSpecificType(new Uint8ClampedArray()),
108+
'an instance of Uint8ClampedArray',
109+
);
110+
strictEqual(
111+
determineSpecificType(new Uint16Array()),
112+
'an instance of Uint16Array',
113+
);
114+
strictEqual(
115+
determineSpecificType(new Uint32Array()),
116+
'an instance of Uint32Array',
117+
);
118+
119+
strictEqual(
120+
determineSpecificType(new Date()),
121+
'an instance of Date',
122+
);
123+
124+
strictEqual(
125+
determineSpecificType(new Map()),
126+
'an instance of Map',
127+
);
128+
strictEqual(
129+
determineSpecificType(new WeakMap()),
130+
'an instance of WeakMap',
131+
);
132+
133+
strictEqual(
134+
determineSpecificType({}),
135+
'an instance of Object',
136+
);
137+
138+
strictEqual(
139+
determineSpecificType(Promise.resolve('foo')),
140+
'an instance of Promise',
141+
);
142+
143+
strictEqual(
144+
determineSpecificType(new Set()),
145+
'an instance of Set',
146+
);
147+
strictEqual(
148+
determineSpecificType(new WeakSet()),
149+
'an instance of WeakSet',
150+
);

‎test/parallel/test-assert-async.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,9 @@ const invalidThenableFunc = () => {
103103
promises.push(assert.rejects(promise, {
104104
name: 'TypeError',
105105
code: 'ERR_INVALID_RETURN_VALUE',
106+
// FIXME(JakobJingleheimer): This should match on key words, like /Promise/ and /undefined/.
106107
message: 'Expected instance of Promise to be returned ' +
107-
'from the "promiseFn" function but got type undefined.'
108+
'from the "promiseFn" function but got undefined.'
108109
}));
109110

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

0 commit comments

Comments
 (0)