Skip to content

Commit

Permalink
assert: improve class instance errors
Browse files Browse the repository at this point in the history
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: #28263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Oct 1, 2019
1 parent eb05d68 commit ace3f16
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 25 deletions.
27 changes: 24 additions & 3 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

'use strict';

const { Object } = primordials;
const { Object, ObjectPrototype } = primordials;

const { Buffer } = require('buffer');
const { codes: {
Expand All @@ -36,6 +36,7 @@ const { inspect } = require('internal/util/inspect');
const { isPromise, isRegExp } = require('internal/util/types');
const { EOL } = require('internal/constants');
const { NativeModule } = require('internal/bootstrap/loaders');
const { isError } = require('internal/util');

const errorCache = new Map();

Expand Down Expand Up @@ -561,6 +562,8 @@ function compareExceptionKey(actual, expected, key, message, keys, fn) {
}

function expectedException(actual, expected, message, fn) {
let generatedMessage = false;

if (typeof expected !== 'function') {
// Handle regular expressions.
if (isRegExp(expected)) {
Expand Down Expand Up @@ -618,8 +621,26 @@ function expectedException(actual, expected, message, fn) {
if (expected.prototype !== undefined && actual instanceof expected) {
return;
}
if (Error.isPrototypeOf(expected)) {
throw actual;
if (ObjectPrototype.isPrototypeOf(Error, expected)) {
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.
Expand Down
71 changes: 49 additions & 22 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,19 @@ assert.throws(() => thrower(a.AssertionError));
assert.throws(() => thrower(TypeError));

// When passing a type, only catch errors of the appropriate type.
{
let threw = false;
try {
a.throws(() => thrower(TypeError), a.AssertionError);
} catch (e) {
threw = true;
assert.ok(e instanceof TypeError, 'type');
assert.throws(
() => a.throws(() => thrower(TypeError), a.AssertionError),
{
generatedMessage: true,
actual: new TypeError({}),
expected: a.AssertionError,
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.
{
Expand Down Expand Up @@ -237,20 +240,27 @@ a.throws(() => thrower(TypeError), (err) => {

// https://github.com/nodejs/node/issues/3188
{
let threw = false;
let AnotherErrorType;
try {
const ES6Error = class extends Error {};
AnotherErrorType = class extends Error {};

assert.throws(() => { throw new AnotherErrorType('foo'); }, ES6Error);
} catch (e) {
threw = true;
assert(e instanceof AnotherErrorType,
`expected AnotherErrorType, received ${e}`);
}
let actual;
assert.throws(
() => {
const ES6Error = class extends Error {};
const AnotherErrorType = class extends Error {};

assert.ok(threw);
assert.throws(() => {
actual = new AnotherErrorType('foo');
throw actual;
}, ES6Error);
},
(err) => {
assert.strictEqual(
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().
Expand Down Expand Up @@ -1299,3 +1309,20 @@ assert.throws(
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]"'
}
);

0 comments on commit ace3f16

Please sign in to comment.