assert: improve class instance errors

This improves `assert.throws()` and `assert.rejects()` in case error
classes are used as validation for the expected error.

In case of a failure it will now throw an `AssertionError` instead
of the received error if the check fails. That error has the received
error as actual property and the expected property is set to the
expected error class.

The error message should help users to identify the problem faster
than before by providing extra context what went wrong.

PR-URL: https://github.com/nodejs/node/pull/28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Ruben Bridgewater 2019-06-14 00:24:31 +02:00
parent eb05d68815
commit ace3f16917
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
2 changed files with 73 additions and 25 deletions

View File

@ -20,7 +20,7 @@
'use strict'; 'use strict';
const { Object } = primordials; const { Object, ObjectPrototype } = primordials;
const { Buffer } = require('buffer'); const { Buffer } = require('buffer');
const { codes: { const { codes: {
@ -36,6 +36,7 @@ const { inspect } = require('internal/util/inspect');
const { isPromise, isRegExp } = require('internal/util/types'); const { isPromise, isRegExp } = require('internal/util/types');
const { EOL } = require('internal/constants'); const { EOL } = require('internal/constants');
const { NativeModule } = require('internal/bootstrap/loaders'); const { NativeModule } = require('internal/bootstrap/loaders');
const { isError } = require('internal/util');
const errorCache = new Map(); const errorCache = new Map();
@ -561,6 +562,8 @@ function compareExceptionKey(actual, expected, key, message, keys, fn) {
} }
function expectedException(actual, expected, message, fn) { function expectedException(actual, expected, message, fn) {
let generatedMessage = false;
if (typeof expected !== 'function') { if (typeof expected !== 'function') {
// Handle regular expressions. // Handle regular expressions.
if (isRegExp(expected)) { if (isRegExp(expected)) {
@ -618,8 +621,26 @@ function expectedException(actual, expected, message, fn) {
if (expected.prototype !== undefined && actual instanceof expected) { if (expected.prototype !== undefined && actual instanceof expected) {
return; return;
} }
if (Error.isPrototypeOf(expected)) { if (ObjectPrototype.isPrototypeOf(Error, expected)) {
throw actual; if (!message) {
generatedMessage = true;
message = 'The error is expected to be an instance of ' +
`"${expected.name}". Received `;
if (isError(actual)) {
message += `"${actual.name}"`;
} else {
message += `"${inspect(actual, { depth: -1 })}"`;
}
}
const err = new AssertionError({
actual,
expected,
message,
operator: fn.name,
stackStartFn: fn
});
err.generatedMessage = generatedMessage;
throw err;
} }
// Check validation functions return value. // Check validation functions return value.

View File

@ -123,16 +123,19 @@ assert.throws(() => thrower(a.AssertionError));
assert.throws(() => thrower(TypeError)); assert.throws(() => thrower(TypeError));
// When passing a type, only catch errors of the appropriate type. // When passing a type, only catch errors of the appropriate type.
{ assert.throws(
let threw = false; () => a.throws(() => thrower(TypeError), a.AssertionError),
try { {
a.throws(() => thrower(TypeError), a.AssertionError); generatedMessage: true,
} catch (e) { actual: new TypeError({}),
threw = true; expected: a.AssertionError,
assert.ok(e instanceof TypeError, 'type'); code: 'ERR_ASSERTION',
name: 'AssertionError',
operator: 'throws',
message: 'The error is expected to be an instance of "AssertionError". ' +
'Received "TypeError"'
} }
assert.ok(threw, 'a.throws with an explicit error is eating extra errors'); );
}
// doesNotThrow should pass through all errors. // doesNotThrow should pass through all errors.
{ {
@ -237,20 +240,27 @@ a.throws(() => thrower(TypeError), (err) => {
// https://github.com/nodejs/node/issues/3188 // https://github.com/nodejs/node/issues/3188
{ {
let threw = false; let actual;
let AnotherErrorType; assert.throws(
try { () => {
const ES6Error = class extends Error {}; const ES6Error = class extends Error {};
AnotherErrorType = class extends Error {}; const AnotherErrorType = class extends Error {};
assert.throws(() => { throw new AnotherErrorType('foo'); }, ES6Error); assert.throws(() => {
} catch (e) { actual = new AnotherErrorType('foo');
threw = true; throw actual;
assert(e instanceof AnotherErrorType, }, ES6Error);
`expected AnotherErrorType, received ${e}`); },
} (err) => {
assert.strictEqual(
assert.ok(threw); err.message,
'The error is expected to be an instance of "ES6Error". ' +
'Received "Error"'
);
assert.strictEqual(err.actual, actual);
return true;
}
);
} }
// Check messages from assert.throws(). // Check messages from assert.throws().
@ -1299,3 +1309,20 @@ assert.throws(
assert(!err2.stack.includes('hidden')); assert(!err2.stack.includes('hidden'));
})(); })();
} }
assert.throws(
() => assert.throws(() => { throw Symbol('foo'); }, RangeError),
{
message: 'The error is expected to be an instance of "RangeError". ' +
'Received "Symbol(foo)"'
}
);
assert.throws(
// eslint-disable-next-line no-throw-literal
() => assert.throws(() => { throw [1, 2]; }, RangeError),
{
message: 'The error is expected to be an instance of "RangeError". ' +
'Received "[Array]"'
}
);