lib: avoid StackOverflow on serializeError

legendecas · RafaelGSS · commit 398674a25a33 · 2025-05-02T17:06:58.000-03:00
`serializeError` should avoid StackOverflow and the test should not rely on `--stack-size`. PR-URL: #58075 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
diff --git a/lib/internal/error_serdes.js b/lib/internal/error_serdes.js
@@ -36,6 +36,7 @@ const kSerializedObject = 1;
 const kInspectedError = 2;
 const kInspectedSymbol = 3;
 const kCustomInspectedObject = 4;
+const kCircularReference = 5;
 
 const kSymbolStringLength = 'Symbol('.length;
 
@@ -44,12 +45,12 @@ const errors = {
 };
 const errorConstructorNames = new SafeSet(ObjectKeys(errors));
 
-function TryGetAllProperties(object, target = object) {
+function TryGetAllProperties(object, target = object, rememberSet) {
   const all = { __proto__: null };
   if (object === null)
     return all;
   ObjectAssign(all,
-               TryGetAllProperties(ObjectGetPrototypeOf(object), target));
+               TryGetAllProperties(ObjectGetPrototypeOf(object), target, rememberSet));
   const keys = ObjectGetOwnPropertyNames(object);
   ArrayPrototypeForEach(keys, (key) => {
     let descriptor;
@@ -68,7 +69,7 @@ function TryGetAllProperties(object, target = object) {
       }
     }
     if (key === 'cause') {
-      descriptor.value = serializeError(descriptor.value);
+      descriptor.value = serializeError(descriptor.value, rememberSet);
       all[key] = descriptor;
     } else if ('value' in descriptor &&
             typeof descriptor.value !== 'function' && typeof descriptor.value !== 'symbol') {
@@ -108,21 +109,27 @@ function inspect(...args) {
 }
 
 let serialize;
-function serializeError(error) {
+function serializeError(error, rememberSet = new SafeSet()) {
   serialize ??= require('v8').serialize;
   if (typeof error === 'symbol') {
     return Buffer.from(StringFromCharCode(kInspectedSymbol) + inspect(error), 'utf8');
   }
+
   try {
     if (typeof error === 'object' &&
         ObjectPrototypeToString(error) === '[object Error]') {
+      if (rememberSet.has(error)) {
+        return Buffer.from([kCircularReference]);
+      }
+      rememberSet.add(error);
+
       const constructors = GetConstructors(error);
       for (let i = 0; i < constructors.length; i++) {
         const name = GetName(constructors[i]);
         if (errorConstructorNames.has(name)) {
           const serialized = serialize({
             constructor: name,
-            properties: TryGetAllProperties(error),
+            properties: TryGetAllProperties(error, error, rememberSet),
           });
           return Buffer.concat([Buffer.from([kSerializedError]), serialized]);
         }
@@ -183,6 +190,11 @@ function deserializeError(error) {
         __proto__: null,
         [customInspectSymbol]: () => fromBuffer(error).toString('utf8'),
       };
+    case kCircularReference:
+      return {
+        __proto__: null,
+        [customInspectSymbol]: () => '[Circular object]',
+      };
   }
   require('assert').fail('This should not happen');
 }
diff --git a/test/sequential/test-error-serdes.js b/test/sequential/test-error-serdes.js
@@ -1,4 +1,4 @@
-// Flags: --expose-internals --stack-size=64
+// Flags: --expose-internals
 'use strict';
 require('../common');
 const assert = require('assert');
@@ -59,7 +59,7 @@ class ErrorWithThowingCause extends Error {
 }
 class ErrorWithCyclicCause extends Error {
   get cause() {
-    return new ErrorWithCyclicCause();
+    return this;
   }
 }
 const errorWithCause = Object
@@ -83,14 +83,18 @@ assert.strictEqual(Object.hasOwn(cycle(errorWithCyclicCause), 'cause'), true);
 assert.deepStrictEqual(cycle(new ErrorWithCause('Error with cause')).cause, new Error('err'));
 assert.strictEqual(cycle(new ErrorWithThowingCause('Error with cause')).cause, undefined);
 assert.strictEqual(Object.hasOwn(cycle(new ErrorWithThowingCause('Error with cause')), 'cause'), false);
-// When the cause is cyclic, it is serialized until Maximum call stack size is reached
+// When the cause is cyclic, it is serialized as a dumb circular reference object.
 let depth = 0;
 let e = cycle(new ErrorWithCyclicCause('Error with cause'));
 while (e.cause) {
   e = e.cause;
   depth++;
 }
-assert(depth > 1);
+assert.strictEqual(depth, 1);
+assert.strictEqual(
+  inspect(cycle(new ErrorWithCyclicCause('Error with cause')).cause),
+  '[Circular object]',
+);
 
 
 {
