Commit d9700fe
lib: avoid StackOverflow on serializeError
`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>1 parent 14a8280 commit d9700fe
2 files changed
Lines changed: 25 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -36,6 +36,7 @@ const kSerializedObject = 1; | |||
| 36 | 36 | const kInspectedError = 2; | |
| 37 | 37 | const kInspectedSymbol = 3; | |
| 38 | 38 | const kCustomInspectedObject = 4; | |
| 39 | + const kCircularReference = 5; | ||
| 39 | 40 | ||
| 40 | 41 | const kSymbolStringLength = 'Symbol('.length; | |
| 41 | 42 | ||
@@ -44,12 +45,12 @@ const errors = { | |||
| 44 | 45 | }; | |
| 45 | 46 | const errorConstructorNames = new SafeSet(ObjectKeys(errors)); | |
| 46 | 47 | ||
| 47 | - function TryGetAllProperties(object, target = object) { | ||
| 48 | + function TryGetAllProperties(object, target = object, rememberSet) { | ||
| 48 | 49 | const all = { __proto__: null }; | |
| 49 | 50 | if (object === null) | |
| 50 | 51 | return all; | |
| 51 | 52 | ObjectAssign(all, | |
| 52 | - TryGetAllProperties(ObjectGetPrototypeOf(object), target)); | ||
| 53 | + TryGetAllProperties(ObjectGetPrototypeOf(object), target, rememberSet)); | ||
| 53 | 54 | const keys = ObjectGetOwnPropertyNames(object); | |
| 54 | 55 | ArrayPrototypeForEach(keys, (key) => { | |
| 55 | 56 | let descriptor; | |
@@ -68,7 +69,7 @@ function TryGetAllProperties(object, target = object) { | |||
| 68 | 69 | } | |
| 69 | 70 | } | |
| 70 | 71 | if (key === 'cause') { | |
| 71 | - descriptor.value = serializeError(descriptor.value); | ||
| 72 | + descriptor.value = serializeError(descriptor.value, rememberSet); | ||
| 72 | 73 | all[key] = descriptor; | |
| 73 | 74 | } else if ('value' in descriptor && | |
| 74 | 75 | typeof descriptor.value !== 'function' && typeof descriptor.value !== 'symbol') { | |
@@ -108,21 +109,27 @@ function inspect(...args) { | |||
| 108 | 109 | } | |
| 109 | 110 | ||
| 110 | 111 | let serialize; | |
| 111 | - function serializeError(error) { | ||
| 112 | + function serializeError(error, rememberSet = new SafeSet()) { | ||
| 112 | 113 | serialize ??= require('v8').serialize; | |
| 113 | 114 | if (typeof error === 'symbol') { | |
| 114 | 115 | return Buffer.from(StringFromCharCode(kInspectedSymbol) + inspect(error), 'utf8'); | |
| 115 | 116 | } | |
| 117 | + | ||
| 116 | 118 | try { | |
| 117 | 119 | if (typeof error === 'object' && | |
| 118 | 120 | ObjectPrototypeToString(error) === '[object Error]') { | |
| 121 | + if (rememberSet.has(error)) { | ||
| 122 | + return Buffer.from([kCircularReference]); | ||
| 123 | + } | ||
| 124 | + rememberSet.add(error); | ||
| 125 | + | ||
| 119 | 126 | const constructors = GetConstructors(error); | |
| 120 | 127 | for (let i = 0; i < constructors.length; i++) { | |
| 121 | 128 | const name = GetName(constructors[i]); | |
| 122 | 129 | if (errorConstructorNames.has(name)) { | |
| 123 | 130 | const serialized = serialize({ | |
| 124 | 131 | constructor: name, | |
| 125 | - properties: TryGetAllProperties(error), | ||
| 132 | + properties: TryGetAllProperties(error, error, rememberSet), | ||
| 126 | 133 | }); | |
| 127 | 134 | return Buffer.concat([Buffer.from([kSerializedError]), serialized]); | |
| 128 | 135 | } | |
@@ -183,6 +190,11 @@ function deserializeError(error) { | |||
| 183 | 190 | __proto__: null, | |
| 184 | 191 | [customInspectSymbol]: () => fromBuffer(error).toString('utf8'), | |
| 185 | 192 | }; | |
| 193 | + case kCircularReference: | ||
| 194 | + return { | ||
| 195 | + __proto__: null, | ||
| 196 | + [customInspectSymbol]: () => '[Circular object]', | ||
| 197 | + }; | ||
| 186 | 198 | } | |
| 187 | 199 | require('assert').fail('This should not happen'); | |
| 188 | 200 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,4 +1,4 @@ | |||
| 1 | - // Flags: --expose-internals --stack-size=64 | ||
| 1 | + // Flags: --expose-internals | ||
| 2 | 2 | 'use strict'; | |
| 3 | 3 | require('../common'); | |
| 4 | 4 | const assert = require('assert'); | |
@@ -59,7 +59,7 @@ class ErrorWithThowingCause extends Error { | |||
| 59 | 59 | } | |
| 60 | 60 | class ErrorWithCyclicCause extends Error { | |
| 61 | 61 | get cause() { | |
| 62 | - return new ErrorWithCyclicCause(); | ||
| 62 | + return this; | ||
| 63 | 63 | } | |
| 64 | 64 | } | |
| 65 | 65 | const errorWithCause = Object | |
@@ -83,14 +83,18 @@ assert.strictEqual(Object.hasOwn(cycle(errorWithCyclicCause), 'cause'), true); | |||
| 83 | 83 | assert.deepStrictEqual(cycle(new ErrorWithCause('Error with cause')).cause, new Error('err')); | |
| 84 | 84 | assert.strictEqual(cycle(new ErrorWithThowingCause('Error with cause')).cause, undefined); | |
| 85 | 85 | assert.strictEqual(Object.hasOwn(cycle(new ErrorWithThowingCause('Error with cause')), 'cause'), false); | |
| 86 | - // When the cause is cyclic, it is serialized until Maximum call stack size is reached | ||
| 86 | + // When the cause is cyclic, it is serialized as a dumb circular reference object. | ||
| 87 | 87 | let depth = 0; | |
| 88 | 88 | let e = cycle(new ErrorWithCyclicCause('Error with cause')); | |
| 89 | 89 | while (e.cause) { | |
| 90 | 90 | e = e.cause; | |
| 91 | 91 | depth++; | |
| 92 | 92 | } | |
| 93 | - assert(depth > 1); | ||
| 93 | + assert.strictEqual(depth, 1); | ||
| 94 | + assert.strictEqual( | ||
| 95 | + inspect(cycle(new ErrorWithCyclicCause('Error with cause')).cause), | ||
| 96 | + '[Circular object]', | ||
| 97 | + ); | ||
| 94 | 98 | ||
| 95 | 99 | ||
| 96 | 100 | { | |
0 commit comments