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: nodejs#27781 Reviewed-By: Rich Trott <[email protected]>
Trott · Jun 13, 2019 · 4a3af65 · 4a3af65
1 parent 8149656
commit 4a3af65
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 18 deletions.
diff --git a/lib/assert.js b/lib/assert.js
@@ -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.
@@ -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
@@ -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) => {