Skip to content

Commit

Permalink
assert: improve regular expression validation
Browse files Browse the repository at this point in the history
This makes sure `assert.throws()` and `assert.rejects()` result in
an easy to understand error message instead of rethrowing the actual
error. This should significantly improve the debugging experience in
case people use an regular expression to validate their errors.

This also adds support for primitive errors that would have caused
runtime errors using the mentioned functions. The input is now
stringified before it's passed to the RegExp to circumvent that.

As drive-by change this also adds some further comments and renames
a variable for clarity.

PR-URL: #27781
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
BridgeAR committed Jun 17, 2019
1 parent 8157a50 commit ef8f147
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 18 deletions.
71 changes: 54 additions & 17 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,30 +549,38 @@ function compareExceptionKey(actual, expected, key, message, keys, fn) {
}
}

function expectedException(actual, expected, msg, fn) {
function expectedException(actual, expected, message, fn) {
if (typeof expected !== 'function') {
if (isRegExp(expected))
return expected.test(actual);
// assert.doesNotThrow does not accept objects.
if (arguments.length === 2) {
throw new ERR_INVALID_ARG_TYPE(
'expected', ['Function', 'RegExp'], expected
);
// Handle regular expressions.
if (isRegExp(expected)) {
const str = String(actual);
if (expected.test(str))
return;

throw new AssertionError({
actual,
expected,
message: message || 'The input did not match the regular expression ' +
`${inspect(expected)}. Input:\n\n${inspect(str)}\n`,
operator: fn.name,
stackStartFn: fn
});
}

// Handle primitives properly.
if (typeof actual !== 'object' || actual === null) {
const err = new AssertionError({
actual,
expected,
message: msg,
message,
operator: 'deepStrictEqual',
stackStartFn: fn
});
err.operator = fn.name;
throw err;
}

// Handle validation objects.
const keys = Object.keys(expected);
// Special handle errors to make sure the name and the message are compared
// as well.
Expand All @@ -589,18 +597,25 @@ function expectedException(actual, expected, msg, fn) {
expected[key].test(actual[key])) {
continue;
}
compareExceptionKey(actual, expected, key, msg, keys, fn);
compareExceptionKey(actual, expected, key, message, keys, fn);
}
return true;
return;
}

// Guard instanceof against arrow functions as they don't have a prototype.
// Check for matching Error classes.
if (expected.prototype !== undefined && actual instanceof expected) {
return true;
return;
}
if (Error.isPrototypeOf(expected)) {
return false;
throw actual;
}

// Check validation functions return value.
const res = expected.call({}, actual);
if (res !== true) {
throw actual;
}
return expected.call({}, actual) === true;
}

function getActual(fn) {
Expand Down Expand Up @@ -695,9 +710,31 @@ function expectsError(stackStartFn, actual, error, message) {
stackStartFn
});
}
if (error && !expectedException(actual, error, message, stackStartFn)) {
throw actual;

if (!error)
return;

expectedException(actual, error, message, stackStartFn);
}

function hasMatchingError(actual, expected) {
if (typeof expected !== 'function') {
if (isRegExp(expected)) {
const str = String(actual);
return expected.test(str);
}
throw new ERR_INVALID_ARG_TYPE(
'expected', ['Function', 'RegExp'], expected
);
}
// Guard instanceof against arrow functions as they don't have a prototype.
if (expected.prototype !== undefined && actual instanceof expected) {
return true;
}
if (Error.isPrototypeOf(expected)) {
return false;
}
return expected.call({}, actual) === true;
}

function expectsNoError(stackStartFn, actual, error, message) {
Expand All @@ -709,7 +746,7 @@ function expectsNoError(stackStartFn, actual, error, message) {
error = undefined;
}

if (!error || expectedException(actual, error)) {
if (!error || hasMatchingError(actual, error)) {
const details = message ? `: ${message}` : '.';
const fnType = stackStartFn.name === 'doesNotReject' ?
'rejection' : 'exception';
Expand Down
22 changes: 21 additions & 1 deletion test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,27 @@ assert.throws(
}

// Use a RegExp to validate the error message.
a.throws(() => thrower(TypeError), /\[object Object\]/);
{
a.throws(() => thrower(TypeError), /\[object Object\]/);

const symbol = Symbol('foo');
a.throws(() => {
throw symbol;
}, /foo/);

a.throws(() => {
a.throws(() => {
throw symbol;
}, /abc/);
}, {
message: 'The input did not match the regular expression /abc/. ' +
"Input:\n\n'Symbol(foo)'\n",
code: 'ERR_ASSERTION',
operator: 'throws',
actual: symbol,
expected: /abc/
});
}

// Use a fn to validate the error object.
a.throws(() => thrower(TypeError), (err) => {
Expand Down

0 comments on commit ef8f147

Please sign in to comment.