Skip to content

Commit

Permalink
fix: closes mochajs#4552
Browse files Browse the repository at this point in the history
Breaks circular references in before objects are serialized to prevent
cryptic error in parallel mode.
  • Loading branch information
sam-super committed Nov 21, 2023
1 parent b41e985 commit 171f77e
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 10 deletions.
16 changes: 6 additions & 10 deletions lib/nodejs/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -188,14 +188,9 @@ class SerializableEvent {
* @param {Array<object|string>} 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<Object>} 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.
Expand Down Expand Up @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
10 changes: 10 additions & 0 deletions test/integration/fixtures/parallel/circular-error.mjs
Original file line number Diff line number Diff line change
@@ -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;
});
});
13 changes: 13 additions & 0 deletions test/integration/parallel.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]');
});
});

0 comments on commit 171f77e

Please sign in to comment.