assert: improve class instance errors

BridgeAR · BridgeAR · commit ace3f169172d · 2019-10-01T22:55:07.000+02:00
This improves `assert.throws()` and `assert.rejects()` in case error classes are used as validation for the expected error. In case of a failure it will now throw an `AssertionError` instead of the received error if the check fails. That error has the received error as actual property and the expected property is set to the expected error class. The error message should help users to identify the problem faster than before by providing extra context what went wrong. PR-URL: #28263 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/assert.js b/lib/assert.js
@@ -20,7 +20,7 @@
 
 'use strict';
 
-const { Object } = primordials;
+const { Object, ObjectPrototype } = primordials;
 
 const { Buffer } = require('buffer');
 const { codes: {
@@ -36,6 +36,7 @@ const { inspect } = require('internal/util/inspect');
 const { isPromise, isRegExp } = require('internal/util/types');
 const { EOL } = require('internal/constants');
 const { NativeModule } = require('internal/bootstrap/loaders');
+const { isError } = require('internal/util');
 
 const errorCache = new Map();
 
@@ -561,6 +562,8 @@ function compareExceptionKey(actual, expected, key, message, keys, fn) {
 }
 
 function expectedException(actual, expected, message, fn) {
+  let generatedMessage = false;
+
   if (typeof expected !== 'function') {
     // Handle regular expressions.
     if (isRegExp(expected)) {
@@ -618,8 +621,26 @@ function expectedException(actual, expected, message, fn) {
   if (expected.prototype !== undefined && actual instanceof expected) {
     return;
   }
-  if (Error.isPrototypeOf(expected)) {
-    throw actual;
+  if (ObjectPrototype.isPrototypeOf(Error, expected)) {
+    if (!message) {
+      generatedMessage = true;
+      message = 'The error is expected to be an instance of ' +
+        `"${expected.name}". Received `;
+      if (isError(actual)) {
+        message += `"${actual.name}"`;
+      } else {
+        message += `"${inspect(actual, { depth: -1 })}"`;
+      }
+    }
+    const err = new AssertionError({
+      actual,
+      expected,
+      message,
+      operator: fn.name,
+      stackStartFn: fn
+    });
+    err.generatedMessage = generatedMessage;
+    throw err;
   }
 
   // Check validation functions return value.
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
@@ -123,16 +123,19 @@ assert.throws(() => thrower(a.AssertionError));
 assert.throws(() => thrower(TypeError));
 
 // When passing a type, only catch errors of the appropriate type.
-{
-  let threw = false;
-  try {
-    a.throws(() => thrower(TypeError), a.AssertionError);
-  } catch (e) {
-    threw = true;
-    assert.ok(e instanceof TypeError, 'type');
+assert.throws(
+  () => a.throws(() => thrower(TypeError), a.AssertionError),
+  {
+    generatedMessage: true,
+    actual: new TypeError({}),
+    expected: a.AssertionError,
+    code: 'ERR_ASSERTION',
+    name: 'AssertionError',
+    operator: 'throws',
+    message: 'The error is expected to be an instance of "AssertionError". ' +
+             'Received "TypeError"'
   }
-  assert.ok(threw, 'a.throws with an explicit error is eating extra errors');
-}
+);
 
 // doesNotThrow should pass through all errors.
 {
@@ -237,20 +240,27 @@ a.throws(() => thrower(TypeError), (err) => {
 
 // https://github.com/nodejs/node/issues/3188
 {
-  let threw = false;
-  let AnotherErrorType;
-  try {
-    const ES6Error = class extends Error {};
-    AnotherErrorType = class extends Error {};
-
-    assert.throws(() => { throw new AnotherErrorType('foo'); }, ES6Error);
-  } catch (e) {
-    threw = true;
-    assert(e instanceof AnotherErrorType,
-           `expected AnotherErrorType, received ${e}`);
-  }
+  let actual;
+  assert.throws(
+    () => {
+      const ES6Error = class extends Error {};
+      const AnotherErrorType = class extends Error {};
 
-  assert.ok(threw);
+      assert.throws(() => {
+        actual = new AnotherErrorType('foo');
+        throw actual;
+      }, ES6Error);
+    },
+    (err) => {
+      assert.strictEqual(
+        err.message,
+        'The error is expected to be an instance of "ES6Error". ' +
+          'Received "Error"'
+      );
+      assert.strictEqual(err.actual, actual);
+      return true;
+    }
+  );
 }
 
 // Check messages from assert.throws().
@@ -1299,3 +1309,20 @@ assert.throws(
     assert(!err2.stack.includes('hidden'));
   })();
 }
+
+assert.throws(
+  () => assert.throws(() => { throw Symbol('foo'); }, RangeError),
+  {
+    message: 'The error is expected to be an instance of "RangeError". ' +
+             'Received "Symbol(foo)"'
+  }
+);
+
+assert.throws(
+  // eslint-disable-next-line no-throw-literal
+  () => assert.throws(() => { throw [1, 2]; }, RangeError),
+  {
+    message: 'The error is expected to be an instance of "RangeError". ' +
+             'Received "[Array]"'
+  }
+);
