assert: handle errors properly with deep*Equal

Author: BridgeAR

doc/api/assert.md

@@ -47,12 +47,12 @@ Only [enumerable "own" properties][] are considered. The
 non-enumerable properties — for such checks, consider using
 [`assert.deepStrictEqual()`][] instead. This can lead to some
 potentially surprising results. For example, the following example does not
-throw an `AssertionError` because the properties on the [`Error`][] object are
+throw an `AssertionError` because the properties on the [`RegExp`][] object are
 not enumerable:
 
 ```js
 // WARNING: This does not throw an AssertionError!
-assert.deepEqual(Error('a'), Error('b'));
+assert.deepEqual(/a/gi, new Date());
 ```
 
 An exception is made for [`Map`][] and [`Set`][]. Maps and Sets have their
@@ -104,6 +104,9 @@ parameter is undefined, a default error message is assigned.
 <!-- YAML
 added: v1.2.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12142
+    description: Error names and messages are now properly compared
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/12142
     description: Set and Map content is also compared

lib/assert.js

@@ -199,6 +199,13 @@ function strictDeepEqual(actual, expected) {
     if (!areSimilarRegExps(actual, expected)) {
       return false;
     }
+  } else if (actualTag === '[object Error]') {
+    // Do not compare the stack as it might differ even though the error itself
+    // is otherwise identical. The non-enumerable name should be identical as
+    // the prototype is also identical. Otherwise this is caught later on.
+    if (actual.message !== expected.message) {
+      return false;
+    }
   } else if (!isFloatTypedArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
     if (!areSimilarTypedArrays(actual, expected)) {
       return false;
@@ -230,6 +237,10 @@ function looseDeepEqual(actual, expected) {
   if (isRegExp(actual) && isRegExp(expected)) {
     return areSimilarRegExps(actual, expected);
   }
+  if (actual instanceof Error && expected instanceof Error) {
+    if (actual.message !== expected.message || actual.name !== expected.name)
+      return false;
+  }
   const actualTag = objectToString(actual);
   const expectedTag = objectToString(expected);
   if (actualTag === expectedTag) {

test/parallel/test-assert-deep.js

@@ -129,23 +129,23 @@ function assertDeepAndStrictEqual(a, b) {
   assert.deepStrictEqual(b, a);
 }
 
-function assertNotDeepOrStrict(a, b) {
-  assert.throws(() => assert.deepEqual(a, b), re`${a} deepEqual ${b}`);
-  assert.throws(() => assert.deepStrictEqual(a, b),
+function assertNotDeepOrStrict(a, b, err) {
+  assert.throws(() => assert.deepEqual(a, b), err || re`${a} deepEqual ${b}`);
+  assert.throws(() => assert.deepStrictEqual(a, b), err ||
                 re`${a} deepStrictEqual ${b}`);
 
-  assert.throws(() => assert.deepEqual(b, a), re`${b} deepEqual ${a}`);
-  assert.throws(() => assert.deepStrictEqual(b, a),
+  assert.throws(() => assert.deepEqual(b, a), err || re`${b} deepEqual ${a}`);
+  assert.throws(() => assert.deepStrictEqual(b, a), err ||
                 re`${b} deepStrictEqual ${a}`);
 }
 
-function assertOnlyDeepEqual(a, b) {
+function assertOnlyDeepEqual(a, b, err) {
   assert.doesNotThrow(() => assert.deepEqual(a, b));
-  assert.throws(() => assert.deepStrictEqual(a, b),
+  assert.throws(() => assert.deepStrictEqual(a, b), err ||
                 re`${a} deepStrictEqual ${b}`);
 
   assert.doesNotThrow(() => assert.deepEqual(b, a));
-  assert.throws(() => assert.deepStrictEqual(b, a),
+  assert.throws(() => assert.deepStrictEqual(b, a), err ||
                 re`${b} deepStrictEqual ${a}`);
 }
 
@@ -469,4 +469,16 @@ assertOnlyDeepEqual(
 assertDeepAndStrictEqual([1, , , 3], [1, , , 3]);
 assertOnlyDeepEqual([1, , , 3], [1, , , 3, , , ]);
 
+// Handle different error messages
+{
+  const err1 = new Error('foo1');
+  const err2 = new Error('foo2');
+  const err3 = new TypeError('foo1');
+  assertNotDeepOrStrict(err1, err2, assert.AssertionError);
+  assertNotDeepOrStrict(err1, err3, assert.AssertionError);
+  // TODO: evaluate if this should throw or not. The same applies for RegExp
+  // Date and any object that has the same keys but not the same prototype.
+  assertOnlyDeepEqual(err1, {}, assert.AssertionError);
+}
+
 /* eslint-enable */