assert: implement partial error comparison

assert.partialDeepStrictEqual now also handled error properties as
expected. On top of that, the main implementation also handles
non-string `name` and `message` properties and the comparison is a
tad faster by removing duplicated comparison steps.

As a drive-by fix this also cleans up some code by abstracting code
and renaming variables for clarity.

PR-URL: https://github.com/nodejs/node/pull/57370
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

lib/internal/util/comparisons.js

@@ -10,17 +10,17 @@ const {
   Error,
   NumberIsNaN,
   NumberPrototypeValueOf,
-  ObjectGetOwnPropertySymbols,
+  ObjectGetOwnPropertySymbols: getOwnSymbols,
   ObjectGetPrototypeOf,
   ObjectIs,
   ObjectKeys,
-  ObjectPrototypeHasOwnProperty,
+  ObjectPrototypeHasOwnProperty: hasOwn,
-  ObjectPrototypePropertyIsEnumerable,
+  ObjectPrototypePropertyIsEnumerable: hasEnumerable,
   ObjectPrototypeToString,
   SafeSet,
   StringPrototypeValueOf,
   SymbolPrototypeValueOf,
-  TypedArrayPrototypeGetByteLength,
+  TypedArrayPrototypeGetByteLength: getByteLength,
   TypedArrayPrototypeGetSymbolToStringTag,
   Uint8Array,
 } = primordials;
@@ -78,8 +78,8 @@ function areSimilarRegExps(a, b) {
 }
 
 function isPartialUint8Array(a, b) {
-  const lenA = TypedArrayPrototypeGetByteLength(a);
+  const lenA = getByteLength(a);
-  const lenB = TypedArrayPrototypeGetByteLength(b);
+  const lenB = getByteLength(b);
   if (lenA < lenB) {
     return false;
   }
@@ -107,10 +107,10 @@ function isPartialArrayBufferView(a, b) {
 }
 
 function areSimilarFloatArrays(a, b) {
-  if (a.byteLength !== b.byteLength) {
+  const len = getByteLength(a);
+  if (len !== getByteLength(b)) {
     return false;
   }
-  const len = TypedArrayPrototypeGetByteLength(a);
   for (let offset = 0; offset < len; offset++) {
     if (a[offset] !== b[offset]) {
       return false;
@@ -158,6 +158,12 @@ function isEqualBoxedPrimitive(val1, val2) {
   assert.fail(`Unknown boxed type ${val1}`);
 }
 
+function isEnumerableOrIdentical(val1, val2, prop, mode, memos, method) {
+  return hasEnumerable(val2, prop) || // This is handled by Object.keys()
+      (mode === kPartial && (val2[prop] === undefined || (prop === 'message' && val2[prop] === ''))) ||
+      innerDeepEqual(val1[prop], val2[prop], mode, memos);
+}
+
 // Notes: Type tags are historical [[Class]] properties that can be set by
 // FunctionTemplate::SetClassName() in C++ or Symbol.toStringTag in JS
 // and retrieved using Object.prototype.toString.call(obj) in JS
@@ -263,8 +269,7 @@ function innerDeepEqual(val1, val2, mode, memos) {
         (val1.size !== val2.size && (mode !== kPartial || val1.size < val2.size))) {
       return false;
     }
-    const result = keyCheck(val1, val2, mode, memos, kIsSet);
+    return keyCheck(val1, val2, mode, memos, kIsSet);
-    return result;
   } else if (isMap(val1)) {
     if (!isMap(val2) ||
         (val1.size !== val2.size && (mode !== kPartial || val1.size < val2.size))) {
@@ -285,26 +290,15 @@ function innerDeepEqual(val1, val2, mode, memos) {
   } else if (isError(val1)) {
     // Do not compare the stack as it might differ even though the error itself
     // is otherwise identical.
-    if (!isError(val2)) {
+    if (!isError(val2) ||
+        !isEnumerableOrIdentical(val1, val2, 'message', mode, memos) ||
+        !isEnumerableOrIdentical(val1, val2, 'name', mode, memos) ||
+        !isEnumerableOrIdentical(val1, val2, 'cause', mode, memos) ||
+        !isEnumerableOrIdentical(val1, val2, 'errors', mode, memos)) {
       return false;
     }
-
+    const hasOwnVal2Cause = hasOwn(val2, 'cause');
-    const message1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'message');
+    if ((hasOwnVal2Cause !== hasOwn(val1, 'cause') && (mode !== kPartial || hasOwnVal2Cause))) {
-    const name1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'name');
-    // TODO(BridgeAR): Adjust cause and errors properties for partial mode.
-    const cause1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'cause');
-    const errors1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'errors');
-
-    if ((message1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'message') ||
-          (!message1Enumerable && val1.message !== val2.message)) ||
-        (name1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'name') ||
-          (!name1Enumerable && val1.name !== val2.name)) ||
-        (cause1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'cause') ||
-          (!cause1Enumerable && (
-            ObjectPrototypeHasOwnProperty(val1, 'cause') !== ObjectPrototypeHasOwnProperty(val2, 'cause') ||
-            !innerDeepEqual(val1.cause, val2.cause, mode, memos)))) ||
-        (errors1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'errors') ||
-         (!errors1Enumerable && !innerDeepEqual(val1.errors, val2.errors, mode, memos)))) {
       return false;
     }
   } else if (isBoxedPrimitive(val1)) {
@@ -348,10 +342,7 @@ function innerDeepEqual(val1, val2, mode, memos) {
 }
 
 function getEnumerables(val, keys) {
-  return ArrayPrototypeFilter(
+  return ArrayPrototypeFilter(keys, (key) => hasEnumerable(val, key));
-    keys,
-    (k) => ObjectPrototypePropertyIsEnumerable(val, k),
-  );
 }
 
 function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
@@ -371,7 +362,7 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
   // Cheap key test
   if (keys2.length > 0) {
     for (const key of keys2) {
-      if (!ObjectPrototypePropertyIsEnumerable(val1, key)) {
+      if (!hasEnumerable(val1, key)) {
         return false;
       }
     }
@@ -380,11 +371,11 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
   if (!isArrayLikeObject) {
     // The pair must have the same number of owned properties.
     if (mode === kPartial) {
-      const symbolKeys = ObjectGetOwnPropertySymbols(val2);
+      const symbolKeys = getOwnSymbols(val2);
       if (symbolKeys.length !== 0) {
         for (const key of symbolKeys) {
-          if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
+          if (hasEnumerable(val2, key)) {
-            if (!ObjectPrototypePropertyIsEnumerable(val1, key)) {
+            if (!hasEnumerable(val1, key)) {
               return false;
             }
             ArrayPrototypePush(keys2, key);
@@ -396,27 +387,27 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
     }
 
     if (mode === kStrict) {
-      const symbolKeysA = ObjectGetOwnPropertySymbols(val1);
+      const symbolKeysA = getOwnSymbols(val1);
       if (symbolKeysA.length !== 0) {
         let count = 0;
         for (const key of symbolKeysA) {
-          if (ObjectPrototypePropertyIsEnumerable(val1, key)) {
+          if (hasEnumerable(val1, key)) {
-            if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
+            if (!hasEnumerable(val2, key)) {
               return false;
             }
             ArrayPrototypePush(keys2, key);
             count++;
-          } else if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
+          } else if (hasEnumerable(val2, key)) {
             return false;
           }
         }
-        const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
+        const symbolKeysB = getOwnSymbols(val2);
         if (symbolKeysA.length !== symbolKeysB.length &&
             getEnumerables(val2, symbolKeysB).length !== count) {
           return false;
         }
       } else {
-        const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
+        const symbolKeysB = getOwnSymbols(val2);
         if (symbolKeysB.length !== 0 &&
             getEnumerables(val2, symbolKeysB).length !== 0) {
           return false;
@@ -441,7 +432,6 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
       c: undefined,
       d: undefined,
       deep: false,
-      deleteFailures: false,
     };
     return objEquiv(val1, val2, mode, keys2, memos, iterationType);
   }
@@ -476,27 +466,21 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
 
   const areEq = objEquiv(val1, val2, mode, keys2, memos, iterationType);
 
-  if (areEq || memos.deleteFailures) {
+  set.delete(val1);
-    set.delete(val1);
+  set.delete(val2);
-    set.delete(val2);
-  }
 
   return areEq;
 }
 
 function setHasEqualElement(set, val1, mode, memo) {
-  const { deleteFailures } = memo;
-  memo.deleteFailures = true;
   for (const val2 of set) {
     if (innerDeepEqual(val1, val2, mode, memo)) {
       // Remove the matching element to make sure we do not check that again.
       set.delete(val2);
-      memo.deleteFailures = deleteFailures;
       return true;
     }
   }
 
-  memo.deleteFailures = deleteFailures;
   return false;
 }
 
@@ -557,6 +541,8 @@ function partialObjectSetEquiv(a, b, mode, set, memo) {
       return false;
     }
   }
+  /* c8 ignore next */
+  assert.fail('Unreachable code');
 }
 
 function setObjectEquiv(a, b, mode, set, memo) {
@@ -623,18 +609,14 @@ function mapHasEqualEntry(set, map, key1, item1, mode, memo) {
   // To be able to handle cases like:
   //   Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
   // ... we need to consider *all* matching keys, not just the first we find.
-  const { deleteFailures } = memo;
-  memo.deleteFailures = true;
   for (const key2 of set) {
     if (innerDeepEqual(key1, key2, mode, memo) &&
       innerDeepEqual(item1, map.get(key2), mode, memo)) {
       set.delete(key2);
-      memo.deleteFailures = deleteFailures;
       return true;
     }
   }
 
-  memo.deleteFailures = deleteFailures;
   return false;
 }
 
@@ -652,6 +634,8 @@ function partialObjectMapEquiv(a, b, mode, set, memo) {
       return false;
     }
   }
+  /* c8 ignore next */
+  assert.fail('Unreachable code');
 }
 
 function mapObjectEquivalence(a, b, mode, set, memo) {
@@ -747,11 +731,11 @@ function partialSparseArrayEquiv(a, b, mode, memos, startA, startB) {
 function partialArrayEquiv(a, b, mode, memos) {
   let aPos = 0;
   for (let i = 0; i < b.length; i++) {
-    let isSparse = b[i] === undefined && !ObjectPrototypeHasOwnProperty(b, i);
+    let isSparse = b[i] === undefined && !hasOwn(b, i);
     if (isSparse) {
       return partialSparseArrayEquiv(a, b, mode, memos, aPos, i);
     }
-    while (!(isSparse = a[aPos] === undefined && !ObjectPrototypeHasOwnProperty(a, aPos)) &&
+    while (!(isSparse = a[aPos] === undefined && !hasOwn(a, aPos)) &&
            !innerDeepEqual(a[aPos], b[i], mode, memos)) {
       aPos++;
       if (aPos > a.length - b.length + i) {
@@ -776,8 +760,7 @@ function sparseArrayEquiv(a, b, mode, memos, i) {
   }
   for (; i < keysA.length; i++) {
     const key = keysA[i];
-    if (!ObjectPrototypeHasOwnProperty(b, key) ||
+    if (!hasOwn(b, key) || !innerDeepEqual(a[key], b[key], mode, memos)) {
-        !innerDeepEqual(a[key], b[key], mode, memos)) {
       return false;
     }
   }
@@ -802,8 +785,8 @@ function objEquiv(a, b, mode, keys2, memos, iterationType) {
       if (!innerDeepEqual(a[i], b[i], mode, memos)) {
         return false;
       }
-      const isSparseA = a[i] === undefined && !ObjectPrototypeHasOwnProperty(a, i);
+      const isSparseA = a[i] === undefined && !hasOwn(a, i);
-      const isSparseB = b[i] === undefined && !ObjectPrototypeHasOwnProperty(b, i);
+      const isSparseB = b[i] === undefined && !hasOwn(b, i);
       if (isSparseA !== isSparseB) {
         return false;
       }

test/parallel/test-assert-deep-with-error.js

@@ -3,6 +3,13 @@ require('../common');
 const assert = require('assert');
 const { test } = require('node:test');
 
+// Disable colored output to prevent color codes from breaking assertion
+// message comparisons. This should only be an issue when process.stdout
+// is a TTY.
+if (process.stdout.isTTY) {
+  process.env.NODE_DISABLE_COLORS = '1';
+}
+
 const defaultStartMessage = 'Expected values to be strictly deep-equal:\n' +
   '+ actual - expected\n' +
   '\n';

test/parallel/test-assert-partial-deep-equal.js

@@ -176,10 +176,32 @@ describe('Object Comparison Tests', () => {
         },
         {
           description:
-            'throws when comparing two objects with different Error instances',
+            'throws when comparing two objects with different Error message',
           actual: { error: new Error('Test error 1') },
           expected: { error: new Error('Test error 2') },
         },
+        {
+          description:
+            'throws when comparing two objects with missing cause on the actual Error',
+          actual: { error: new Error('Test error 1') },
+          expected: { error: new Error('Test error 1', { cause: 42 }) },
+        },
+        {
+          description:
+            'throws when comparing two objects with missing message on the actual Error',
+          actual: { error: new Error() },
+          expected: { error: new Error('Test error 1') },
+        },
+        {
+          description: 'throws when comparing two Errors with missing cause on the actual Error',
+          actual: { error: new Error('Test error 1') },
+          expected: { error: new Error('Test error 1', { cause: undefined }) },
+        },
+        {
+          description: 'throws when comparing two AggregateErrors with missing message on the actual Error',
+          actual: { error: new AggregateError([], 'Test error 1') },
+          expected: { error: new AggregateError([new Error()], 'Test error 1') },
+        },
         {
           description:
             'throws when comparing two objects with different TypedArray instances and content',
@@ -1105,6 +1127,26 @@ describe('Object Comparison Tests', () => {
           }
         }]
       },
+      {
+        description: 'comparing two Errors with missing cause on the expected Error',
+        actual: { error: new Error('Test error 1', { cause: 42 }) },
+        expected: { error: new Error('Test error 1') },
+      },
+      {
+        description: 'comparing two Errors with cause set to undefined on the actual Error',
+        actual: { error: new Error('Test error 1', { cause: undefined }) },
+        expected: { error: new Error('Test error 1') },
+      },
+      {
+        description: 'comparing two Errors with missing message on the expected Error',
+        actual: { error: new Error('Test error 1') },
+        expected: { error: new Error() },
+      },
+      {
+        description: 'comparing two AggregateErrors with no message or errors on the expected Error',
+        actual: { error: new AggregateError([new Error(), 123]) },
+        expected: { error: new AggregateError([]) },
+      },
     ].forEach(({ description, actual, expected }) => {
       it(description, () => {
         assert.partialDeepStrictEqual(actual, expected);