assert: improve assertion errors

doc/api/assert.md

@@ -1210,10 +1210,14 @@ assert.throws(
   () => {
     throw new Error('Wrong value');
   },
-  function(err) {
-    if ((err instanceof Error) && /value/.test(err)) {
-      return true;
-    }
+  (err) => {
+    assert(err instanceof Error);
+    assert(/value/.test(err));
+    // Returning anything from validation functions besides `true` is not
+    // recommended. Doing so results in the caught error being thrown again.
+    // That is mostly not the desired outcome. Throw an error about the specific
+    // validation that failed instead (as done in this example).
+    return true;
   },
   'unexpected error'
 );

lib/assert.js

@@ -269,24 +269,31 @@ function getErrMessage(message, fn) {
   Error.prepareStackTrace = tmpPrepare;
 
   const filename = call.getFileName();
-
-  if (!filename) {
-    return message;
-  }
-
   const line = call.getLineNumber() - 1;
   let column = call.getColumnNumber() - 1;
+  let identifier;
+  let code;
 
-  const identifier = `${filename}${line}${column}`;
+  if (filename) {
+    identifier = `${filename}${line}${column}`;
 
-  if (errorCache.has(identifier)) {
-    return errorCache.get(identifier);
+    // Skip Node.js modules!
+    if (filename.endsWith('.js') &&
+        NativeModule.exists(filename.slice(0, -3))) {
+      errorCache.set(identifier, undefined);
+      return;
+    }
+  } else {
+    const fn = call.getFunction();
+    if (!fn) {
+      return message;
+    }
+    code = String(fn);
+    identifier = `${code}${line}${column}`;
   }
 
-  // Skip Node.js modules!
-  if (filename.endsWith('.js') && NativeModule.exists(filename.slice(0, -3))) {
-    errorCache.set(identifier, undefined);
-    return;
+  if (errorCache.has(identifier)) {
+    return errorCache.get(identifier);
   }
 
   let fd;
@@ -295,16 +302,22 @@ function getErrMessage(message, fn) {
     // errors are handled faster.
     Error.stackTraceLimit = 0;
 
-    if (decoder === undefined) {
-      const { StringDecoder } = require('string_decoder');
-      decoder = new StringDecoder('utf8');
+    if (filename) {
+      if (decoder === undefined) {
+        const { StringDecoder } = require('string_decoder');
+        decoder = new StringDecoder('utf8');
+      }
+      fd = openSync(filename, 'r', 0o666);
+      // Reset column and message.
+      [column, message] = getCode(fd, line, column);
+      // Flush unfinished multi byte characters.
+      decoder.end();
+    } else {
+      for (let i = 0; i < line; i++) {
+        code = code.slice(code.indexOf('\n') + 1);
+      }
+      [column, message] = parseCode(code, column);
     }
-
-    fd = openSync(filename, 'r', 0o666);
-    // Reset column and message.
-    [column, message] = getCode(fd, line, column);
-    // Flush unfinished multi byte characters.
-    decoder.end();
     // Always normalize indentation, otherwise the message could look weird.
     if (message.includes('\n')) {
       if (EOL === '\r\n') {
@@ -549,30 +562,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 +610,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 +723,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 +759,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';

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) => {
@@ -834,6 +854,13 @@ common.expectsError(
   {
     code: 'ERR_ASSERTION',
     type: assert.AssertionError,
+    message: 'The expression evaluated to a falsy value:\n\n  assert(1 === 2)\n'
+  }
+);
+assert.throws(
+  () => eval('console.log("FOO");\nassert.ok(1 === 2);'),
+  {
+    code: 'ERR_ASSERTION',
     message: 'false == true'
   }
 );