assert: handle errors properly with deep*Equal

PR-URL: #15001 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Sep 12, 2017 · bcf0e5d · bcf0e5d
1 parent 2aec977
commit bcf0e5d
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 10 deletions.
diff --git a/doc/api/assert.md b/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

diff --git a/lib/assert.js b/lib/assert.js
@@ -197,6 +197,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;
@@ -228,6 +235,10 @@ function looseDeepEqual(actual, expected) {
   if (util.isRegExp(actual) && util.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) {

diff --git a/test/parallel/test-assert-deep.js b/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}`);
 }
 
@@ -407,4 +407,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 */