Repository: jestjs/jest (TypeScript)

Issue:
Circular references hang jest when assertions fail on node 14
<!-- Love Jest? Please consider supporting our collective: 👉  https://opencollective.com/jest/donate -->

## 🐛 Bug Report
When an assertion fails where either the expected or actual value is circular, and both values are objects, jest encounters an error stating it failed to convert a circular structure to JSON, resulting in the test run not completing.

## To Reproduce
```ts
it("test", () => {
  const foo = {};
  foo.ref = foo;

  expect(foo).toEqual({});
});
```

Running jest gives me the following error:
```
(node:11685) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    --- property 'ref' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (internal/child_process/serialization.js:117:20)
    at process.target._send (internal/child_process.js:804:17)
    at process.target.send (internal/child_process.js:702:19)
    at reportSuccess (/Users/verit/basic-jsx/node_modules/jest-worker/build/workers/processChild.js:67:11)
```
Jest continues running indefinitely (I only tested up to ten minutes) and reports nothing regarding the test suite.

I traced this to the [added `failureDetails` property on error messages](https://github.com/facebook/jest/pull/9496/files#diff-0755289e4b7e144aa0ef6f59f5bcf748R436), landed in [26.3.0](https://github.com/facebook/jest/releases/tag/v26.3.0).

## Expected behavior
I'd expect the test to fail and jest to complete running.

## envinfo
I only tested two versions. The above error occurs on 14.9.0, but does not on 12.16.1.

```
  System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i7-7920HQ CPU @ 3.10GHz
  Binaries:
    Node: 14.9.0 - ~/.nodenv/versions/14.9.0/bin/node
    npm: 6.14.8 - ~/.nodenv/versions/14.9.0/bin/npm
  npmPackages:
    jest: ^26.4.2 => 26.4.2 
```

fix(jest-worker): Remove circular references from messages sent to workers
Fixes #10577

<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->

<!-- Please remember to update CHANGELOG.md at the root of the project if you have not done so. -->

## Summary

This fix prevents objects with circular references to be sent to worker processes. Node uses `JSON.stringify` to serialize such messages and this causes an error in case there are circular references in the messages.

Please note I added new dependency, `fclone`, though I'm not sure it's allowed or if there already is another such function in the repository.

## Test plan

This fix allows to run the tests described in the issue linked. The repro setup can be found at https://repl.it/@Frantiekiaik/jest-playground-1.




Hints:
Is there any workaround?
@Lonli-Lokli Looks like the only workaround for now is `--detectOpenHandles `

However, this causes a massive decrease in performance. 

I hope this can be fixed soon. I have migrated several of our projects to jest... and now it causes hiccups all throughout our build system when one test breaks. 

It would be nice if `--testTimeout` worked in this scenario... but it still allows the test to just hang until Jenkins or circle ci times out. 
I can reproduce this when I have two such tests (in separate files - but not sure if this matters) and I have to run it with `--watch`. Here, however, it can be reproduced even without `--watch`: https://repl.it/@Frantiekiaik/jest-playground-1

I just ran into what I assume is the same issue on Node 10.16.0 and Jest 26.6.2. I can't reproduce it as written, but if I make a file containing two copies of @voces's example test and run it in watch mode, I get the same error.

The test:
```js
it('test', () => {
  const foo = {};
  foo.ref = foo;

  expect(foo).toEqual({});
});
it('test 2', () => {
  const foo = {};
  foo.ref = foo;

  expect(foo).toEqual({});
});
```

The error:
```
(node:58951) ExperimentalWarning: The fs.promises API is experimental
(node:58951) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at process.target._send (internal/child_process.js:735:23)
    at process.target.send (internal/child_process.js:634:19)
    at reportSuccess (/Users/richardmunroe/analytics_ui/node_modules/jest-worker/build/workers/processChild.js:67:11)
(node:58951) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:58951) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
```

Running with `--detectOpenHandles` makes the problem go away for me too.
This is happening since #9496 
Commit 918636142791a3dd4ddfe9367149a90437bd6da9

And it seems to happen because of some inter-process serialization. After adding `failureDetails` property, the serialization fails on cyclic references as it contains the objects being under test. Maybe some sanitization of the `failureDetails` property would help.
Note that after commit 5f6f2ec8e17555b695d65ab68824926c77730216 which changes default runner to circus, the error message is 
```
"messageParent" can only be used inside a worker

      at messageParent (packages/jest-worker/build/workers/messageParent.js:46:11)
```

with `JEST_JASMINE=1` the error is as before.
Added a PR as an attempt to fix this.

Also, please note that the `messageParent` error mentioned above is due to [this line](https://github.com/facebook/jest/blob/245a58278ce189019e4af3537b0f987bf9ff210e/packages/jest-worker/src/workers/messageParent.ts#L33) which swallows the real error message which is also about circular references.

> Added a PR as an attempt to fix this.

🎉 

> Also, please note that the `messageParent` error mentioned above is due to [this line](https://github.com/facebook/jest/blob/245a58278ce189019e4af3537b0f987bf9ff210e/packages/jest-worker/src/workers/messageParent.ts#L33) which swallows the real error message which is also about circular references.

We should not swallow errors like that...
We confirm it happens in Node 12, and it’s more common when using Angular Dependency Injection (I think they have cyclic structures in some error-states).

The process hangs in such scenario, but this can be improved slightly be applying `--unhandled-rejection=strict` to nodejs script, instead running jest as separate binary. It helps `jest` to recover and fail suite (but it does not resolve cyclic reference of course).
I can confirm this problem as very common with Angular DI with Node 12 as @piotrl mentioned. 
Using Jest 26.6.3

How can I run it with `--unhandled-rejection=strict` flag?  

```json
    //package.json > scripts
    "test:unit": "node --unhandled-rejections=strict $(npm bin)/jest --env=jest-environment-jsdom-sixteen ",
```
this is actually stopping the execution, is that the right workaround?
We'd want to restore the original object on the receiving side, no? Should we use something like https://github.com/storybookjs/telejson?
@SimenB sounds reasonable. I'll try to rework.
Would the ['advanced'](https://nodejs.org/api/child_process.html#child_process_advanced_serialization) serialization format be an alternative for use-cases that require circular references to avoid that all workers must pay for the overhead of `fclone`? This would also solve the issue where e.g. a child returns a structure containing circular references. 

I would suggest to not enable this option by default because it comes with a slight performance overhead.
@MichaReiser I tried that, but it just changes the error to the same as worker threads

```
(node:24057) UnhandledPromiseRejectionWarning: Error: () =>
          (0, _jestMatcherUtils.matcherHint)(
            matcherName,
            undefined,
           ...<omitted>... ) could not be cloned.
    at writeChannelMessage (internal/child_process/serialization.js:79:9)
    at process.target._send (internal/child_process.js:805:17)
    at process.target.send (internal/child_process.js:703:19)
    at reportSuccess (/Users/simen/repos/jest/packages/jest-worker/build/workers/processChild.js:67:11)
```

While using worker (which uses structured clone by default)

```
(node:24148) UnhandledPromiseRejectionWarning: DataCloneError: () =>
          (0, _jestMatcherUtils.matcherHint)(
            matcherName,
            undefined,
           ...<omitted>... ) could not be cloned.
    at reportSuccess (/Users/simen/repos/jest/packages/jest-worker/build/workers/threadChild.js:77:32)
```
@SimenB  that's rather interesting because the MDN documentation explicitly mentions circular references...

> The structured clone algorithm copies complex JavaScript objects. It is used internally to transfer data between Workers via postMessage(), storing objects with IndexedDB, or copying objects for other APIs. It clones by recursing through the input object while maintaining a map of previously visited references, **to avoid infinitely traversing cycles**. [source](https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm).

What kind of object did you pass? Any chance it did use Proxies, contained Symbols, or used other features that make it not a plain JS object? [spec](https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal)

However, it is as it is. I still believe that it would make sense to change jest worker to allow passing in a custom serializer / deserialiser instead of requiring a custom serialization logic for all clients. Or manually calling `fclone` before invoking the method on the `jest-worker` / reading the value in the worker. E.g. I personally would prefer to get an error than being surprised by the fact that some properties in the worker/ in the result have been changed to `[Circular]` for no obvious reasons.

Patch:
diff --git a/packages/jest-runner/src/index.ts b/packages/jest-runner/src/index.ts
--- a/packages/jest-runner/src/index.ts
+++ b/packages/jest-runner/src/index.ts
@@ -166,7 +166,12 @@ export default class TestRunner {
 
     const worker = new Worker(TEST_WORKER_PATH, {
       exposedMethods: ['worker'],
-      forkOptions: {stdio: 'pipe'},
+      forkOptions: {
+        // use advanced serialization in order to transfer objects with circular references
+        // @ts-expect-error: option does not exist on the node 10 types
+        serialization: 'advanced',
+        stdio: 'pipe',
+      },
       maxRetries: 3,
       numWorkers: this._globalConfig.maxWorkers,
       setupArgs: [


Test patch:
diff --git a/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap b/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap
new file mode 100644
--- /dev/null
+++ b/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap
@@ -0,0 +1,53 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`handles circular inequality properly 1`] = `
+FAIL __tests__/test-1.js
+  ● test
+
+    expect(received).toEqual(expected) // deep equality
+
+    - Expected  - 1
+    + Received  + 3
+
+    - Object {}
+    + Object {
+    +   "ref": [Circular],
+    + }
+
+      3 |   foo.ref = foo;
+      4 |
+    > 5 |   expect(foo).toEqual({});
+        |               ^
+      6 | });
+
+      at Object.toEqual (__tests__/test-1.js:5:15)
+
+FAIL __tests__/test-2.js
+  ● test
+
+    expect(received).toEqual(expected) // deep equality
+
+    - Expected  - 1
+    + Received  + 3
+
+    - Object {}
+    + Object {
+    +   "ref": [Circular],
+    + }
+
+      3 |   foo.ref = foo;
+      4 |
+    > 5 |   expect(foo).toEqual({});
+        |               ^
+      6 | });
+
+      at Object.toEqual (__tests__/test-2.js:5:15)
+`;
+
+exports[`handles circular inequality properly 2`] = `
+Test Suites: 2 failed, 2 total
+Tests:       2 failed, 2 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+`;
diff --git a/e2e/__tests__/circularInequality.test.ts b/e2e/__tests__/circularInequality.test.ts
new file mode 100644
--- /dev/null
+++ b/e2e/__tests__/circularInequality.test.ts
@@ -0,0 +1,58 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import {tmpdir} from 'os';
+import * as path from 'path';
+import {wrap} from 'jest-snapshot-serializer-raw';
+import {
+  cleanup,
+  createEmptyPackage,
+  extractSortedSummary,
+  writeFiles,
+} from '../Utils';
+import {runContinuous} from '../runJest';
+
+const tempDir = path.resolve(tmpdir(), 'circular-inequality-test');
+
+beforeEach(() => {
+  createEmptyPackage(tempDir);
+});
+
+afterEach(() => {
+  cleanup(tempDir);
+});
+
+test('handles circular inequality properly', async () => {
+  const testFileContent = `
+    it('test', () => {
+      const foo = {};
+      foo.ref = foo;
+
+      expect(foo).toEqual({});
+    });
+  `;
+
+  writeFiles(tempDir, {
+    '__tests__/test-1.js': testFileContent,
+    '__tests__/test-2.js': testFileContent,
+  });
+
+  const {end, waitUntil} = runContinuous(
+    tempDir,
+    ['--no-watchman', '--watch-all'],
+    // timeout in case the `waitUntil` below doesn't fire
+    {stripAnsi: true, timeout: 5000},
+  );
+
+  await waitUntil(({stderr}) => stderr.includes('Ran all test suites.'));
+
+  const {stderr} = await end();
+
+  const {summary, rest} = extractSortedSummary(stderr);
+  expect(wrap(rest)).toMatchSnapshot();
+  expect(wrap(summary)).toMatchSnapshot();
+});