From 171f77ec45d2bd82783c46bc0d4bee32195b95b9 Mon Sep 17 00:00:00 2001 From: Sam Adams Date: Tue, 21 Nov 2023 08:32:14 +0000 Subject: [PATCH 1/2] fix: closes #4552 Breaks circular references in before objects are serialized to prevent cryptic error in parallel mode. --- lib/nodejs/serializer.js | 16 ++++----- lib/utils.js | 33 +++++++++++++++++++ .../fixtures/parallel/circular-error.mjs | 10 ++++++ test/integration/parallel.spec.js | 13 ++++++++ 4 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 test/integration/fixtures/parallel/circular-error.mjs diff --git a/lib/nodejs/serializer.js b/lib/nodejs/serializer.js index b25c493bf0..314c723c13 100644 --- a/lib/nodejs/serializer.js +++ b/lib/nodejs/serializer.js @@ -6,7 +6,7 @@ 'use strict'; -const {type} = require('../utils'); +const {type, breakCircularDeps} = require('../utils'); const {createInvalidArgumentTypeError} = require('../errors'); // this is not named `mocha:parallel:serializer` because it's noisy and it's // helpful to be able to write `DEBUG=mocha:parallel*` and get everything else. @@ -188,14 +188,9 @@ class SerializableEvent { * @param {Array} pairs - List of parent/key tuples to process; modified in-place. This JSDoc type is an approximation * @param {object} parent - Some parent object * @param {string} key - Key to inspect - * @param {WeakSet} seenObjects - For avoiding circular references */ - static _serialize(pairs, parent, key, seenObjects) { + static _serialize(pairs, parent, key) { let value = parent[key]; - if (seenObjects.has(value)) { - parent[key] = Object.create(null); - return; - } let _type = type(value); if (_type === 'error') { // we need to reference the stack prop b/c it's lazily-loaded. @@ -263,13 +258,14 @@ class SerializableEvent { error: this.originalError }); + // mutates the object + breakCircularDeps(result); + const pairs = Object.keys(result).map(key => [result, key]); - const seenObjects = new WeakSet(); let pair; while ((pair = pairs.shift())) { - SerializableEvent._serialize(pairs, ...pair, seenObjects); - seenObjects.add(pair[0]); + SerializableEvent._serialize(pairs, ...pair); } this.data = result.data; diff --git a/lib/utils.js b/lib/utils.js index fc7271d019..4de9d28276 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -647,3 +647,36 @@ exports.assignNewMochaID = obj => { */ exports.getMochaID = obj => obj && typeof obj === 'object' ? obj[MOCHA_ID_PROP_NAME] : undefined; + +/** + * Replaces any detected circular dependency with the string '[Circular]' + * Mutates original object + * @param inputObj {*} + * @returns {*} + */ +exports.breakCircularDeps = inputObj => { + const seen = new Set(); + + function _breakCircularDeps(obj) { + if (obj && typeof obj !== 'object') { + return obj; + } + + if (seen.has(obj)) { + return '[Circular]'; + } + + seen.add(obj); + for (const k in obj) { + if (Object.prototype.hasOwnProperty.call(obj, k)) { + obj[k] = _breakCircularDeps(obj[k], k); + } + } + + // deleting means only a seen object that is its own child will be detected + seen.delete(obj); + return obj; + } + + return _breakCircularDeps(inputObj); +}; diff --git a/test/integration/fixtures/parallel/circular-error.mjs b/test/integration/fixtures/parallel/circular-error.mjs new file mode 100644 index 0000000000..5e3787b83f --- /dev/null +++ b/test/integration/fixtures/parallel/circular-error.mjs @@ -0,0 +1,10 @@ +import {describe,it} from "../../../../index.js"; + +describe('test1', () => { + it('test', () => { + const error = new Error('Foo'); + error.foo = { props: [] }; + error.foo.props.push(error.foo); + throw error; + }); +}); diff --git a/test/integration/parallel.spec.js b/test/integration/parallel.spec.js index 3cdecfcf18..5dc1d07fcd 100644 --- a/test/integration/parallel.spec.js +++ b/test/integration/parallel.spec.js @@ -30,4 +30,17 @@ describe('parallel run', () => { assert.strictEqual(result.stats.failures, 0); assert.strictEqual(result.stats.passes, 3); }); + + it('should correctly handle circular references in an exception', async () => { + const result = await runMochaJSONAsync('parallel/circular-error.mjs', [ + '--parallel', + '--jobs', + '2', + require.resolve('./fixtures/parallel/testworkerid1.mjs') + ]); + assert.strictEqual(result.stats.failures, 1); + assert.strictEqual(result.stats.passes, 1); + assert.strictEqual(result.failures[0].err.message, 'Foo'); + assert.strictEqual(result.failures[0].err.foo.props[0], '[Circular]'); + }); }); From 695b17c73a24c33de57cfa30170c004c656a2e81 Mon Sep 17 00:00:00 2001 From: Sam Adams Date: Fri, 24 Nov 2023 09:25:46 +0000 Subject: [PATCH 2/2] chore: trigger test re-run