Skip to content

Commit 807c108

Browse files
committed
util: improve internal isError() validation
The current internal isError function checked the toString value instead of using the more precise `util.types.isNativeError()` check. The `instanceof` check is not removed due to possible errors that are not native but still an instance of Error. PR-URL: #24746 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent e13571c commit 807c108

File tree

3 files changed

+54
-3
lines changed

3 files changed

+54
-3
lines changed

lib/internal/util.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ const {
1212
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
1313
decorated_private_symbol: kDecoratedPrivateSymbolIndex
1414
} = internalBinding('util');
15+
const {
16+
isNativeError
17+
} = internalBinding('types');
1518

1619
const { errmap } = internalBinding('uv');
1720

@@ -26,7 +29,10 @@ function removeColors(str) {
2629
}
2730

2831
function isError(e) {
29-
return objectToString(e) === '[object Error]' || e instanceof Error;
32+
// An error could be an instance of Error while not being a native error
33+
// or could be from a different realm and not be instance of Error but still
34+
// be a native error.
35+
return isNativeError(e) || e instanceof Error;
3036
}
3137

3238
function objectToString(o) {

lib/util.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,16 @@ const {
4242
const {
4343
deprecate,
4444
getSystemErrorName: internalErrorName,
45-
isError,
4645
promisify,
4746
} = require('internal/util');
4847

48+
const ReflectApply = Reflect.apply;
49+
50+
function uncurryThis(func) {
51+
return (thisArg, ...args) => ReflectApply(func, thisArg, args);
52+
}
53+
const objectToString = uncurryThis(Object.prototype.toString);
54+
4955
let CIRCULAR_ERROR_MESSAGE;
5056
let internalDeepEqual;
5157

@@ -429,7 +435,9 @@ module.exports = exports = {
429435
isRegExp,
430436
isObject,
431437
isDate,
432-
isError,
438+
isError(e) {
439+
return objectToString(e) === '[object Error]' || e instanceof Error;
440+
},
433441
isFunction,
434442
isPrimitive,
435443
log,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
const { types } = require('util');
7+
const { isError } = require('internal/util');
8+
const vm = require('vm');
9+
10+
// Special cased errors. Test the internal function which is used in
11+
// `util.inspect()`, the `repl` and maybe more. This verifies that errors from
12+
// different realms, and non native instances of error are properly detected as
13+
// error while definitely false ones are not detected. This is different than
14+
// the public `util.isError()` function which falsy detects the fake errors as
15+
// actual errors.
16+
{
17+
const fake = { [Symbol.toStringTag]: 'Error' };
18+
assert(!types.isNativeError(fake));
19+
assert(!(fake instanceof Error));
20+
assert(!isError(fake));
21+
22+
const err = new Error('test');
23+
const newErr = Object.create(
24+
Object.getPrototypeOf(err),
25+
Object.getOwnPropertyDescriptors(err));
26+
Object.defineProperty(err, 'message', { value: err.message });
27+
assert(types.isNativeError(err));
28+
assert(!types.isNativeError(newErr));
29+
assert(newErr instanceof Error);
30+
assert(isError(newErr));
31+
32+
const context = vm.createContext({});
33+
const differentRealmErr = vm.runInContext('new Error()', context);
34+
assert(types.isNativeError(differentRealmErr));
35+
assert(!(differentRealmErr instanceof Error));
36+
assert(isError(differentRealmErr));
37+
}

0 commit comments

Comments
 (0)