assert: make sure throws is able to handle primitives

BridgeAR · BridgeAR · commit 34b48183644e · 2018-05-10T13:24:59.000+02:00
This fixes some possible issues with `assert.throws` in combination
with an validation object. It will now properly handle primitive
values being thrown as error.

It also makes sure the `generatedMessage` property is properly set
if `assert.throws` is used in combination with an validation object
and improves the error performance in such cases by only creating
the error once.
diff --git a/lib/assert.js b/lib/assert.js
@@ -385,16 +385,16 @@ function compareExceptionKey(actual, expected, key, message, keys) {
       const a = new Comparison(actual, keys);
       const b = new Comparison(expected, keys, actual);
 
-      const tmpLimit = Error.stackTraceLimit;
-      Error.stackTraceLimit = 0;
       const err = new AssertionError({
         actual: a,
         expected: b,
         operator: 'deepStrictEqual',
         stackStartFn: assert.throws
       });
-      Error.stackTraceLimit = tmpLimit;
-      message = err.message;
+      err.actual = actual;
+      err.expected = expected;
+      err.operator = 'throws';
+      throw err;
     }
     innerFail({
       actual,
@@ -408,14 +408,34 @@ function compareExceptionKey(actual, expected, key, message, keys) {
 
 function expectedException(actual, expected, msg) {
   if (typeof expected !== 'function') {
-    if (expected instanceof RegExp)
+    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
       );
     }
+
+    // TODO: Disallow primitives as error argument.
+    // This is here to prevent a breaking change.
+    if (typeof expected !== 'object') {
+      return true;
+    }
+
+    // Handle primitives properly.
+    if (typeof actual !== 'object' || actual === null) {
+      const err = new AssertionError({
+        actual,
+        expected,
+        message: msg,
+        operator: 'deepStrictEqual',
+        stackStartFn: assert.throws
+      });
+      err.operator = 'throws';
+      throw err;
+    }
+
     const keys = Object.keys(expected);
     // Special handle errors to make sure the name and the message are compared
     // as well.
diff --git a/test/message/assert_throws_stack.out b/test/message/assert_throws_stack.out
@@ -1,6 +1,6 @@
 assert.js:*
-  throw new AssertionError(obj);
-  ^
+      throw err;
+      ^
 
 AssertionError [ERR_ASSERTION]: Input A expected to strictly deep-equal input B:
 + expected - actual
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
@@ -740,7 +740,9 @@ common.expectsError(
     const frames = err.stack.split('\n');
     const [, filename, line, column] = frames[1].match(/\((.+):(\d+):(\d+)\)/);
     // Reset the cache to check again
+    const size = errorCache.size;
     errorCache.delete(`${filename}${line - 1}${column - 1}`);
+    assert.strictEqual(errorCache.size, size - 1);
     const data = `${'\n'.repeat(line - 1)}${' '.repeat(column - 1)}` +
                  'ok(failed(badly));';
     try {
@@ -849,6 +851,7 @@ common.expectsError(
     {
       name: 'AssertionError [ERR_ASSERTION]',
       code: 'ERR_ASSERTION',
+      generatedMessage: true,
       message: `${start}\n${actExp}\n\n` +
                "  Comparison {\n    name: 'Error',\n-   message: 'foo'" +
                "\n+   message: ''\n  }"
@@ -940,3 +943,45 @@ assert.throws(
              '  }'
   }
 );
+
+{
+  let actual = null;
+  const expected = { message: 'foo' };
+  assert.throws(
+    () => assert.throws(
+      () => { throw actual; },
+      expected
+    ),
+    {
+      operator: 'throws',
+      actual,
+      expected,
+      generatedMessage: true,
+      message: `${start}\n${actExp}\n\n` +
+              '- null\n' +
+              '+ {\n' +
+              "+   message: 'foo'\n" +
+              '+ }'
+    }
+  );
+
+  actual = 'foobar';
+  const message = 'message';
+  assert.throws(
+    () => assert.throws(
+      () => { throw actual; },
+      { message: 'foobar' },
+      message
+    ),
+    {
+      actual,
+      message,
+      operator: 'throws',
+      generatedMessage: false
+    }
+  );
+}
+
+// TODO: This case is only there to make sure there is no breaking change.
+// eslint-disable-next-line no-restricted-syntax, no-throw-literal
+assert.throws(() => { throw 4; }, 4);
