assert: improve regular expression validation

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: https://github.com/nodejs/node/pull/27781
Reviewed-By: Rich Trott <rtrott@gmail.com>

2 files changed, 75 insertions, 18 deletions

diff --git a/lib/assert.js b/lib/assert.js
index 6e0b850b40..fadc3ad530 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -549,15 +549,22 @@ 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.
@@ -565,7 +572,7 @@ function expectedException(actual, expected, msg, fn) {
       const err = new AssertionError({
         actual,
         expected,
-        message: msg,
+        message,
         operator: 'deepStrictEqual',
         stackStartFn: fn
       });
@@ -573,6 +580,7 @@ function expectedException(actual, expected, msg, fn) {
       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.
@@ -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) {
@@ -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) {
@@ -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';
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index 52e8572479..af2b683bfa 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -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) => {