fix(worker): handle circular messages

SimenB · SimenB · commit 5e8ae67371d2 · 2020-12-30T17:15:15.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,7 @@
 - `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options ([#10834](https://github.com/facebook/jest/pull/10834))
 - `[jest-worker]` [**BREAKING**] Use named exports ([#10623] (https://github.com/facebook/jest/pull/10623))
 - `[jest-worker]` Do not swallow errors during serialization ([#10984] (https://github.com/facebook/jest/pull/10984))
+- `[jest-worker]` Handle passing messages with circular data ([#10981] (https://github.com/facebook/jest/pull/10981))
 - `[pretty-format]` [**BREAKING**] Convert to ES Modules ([#10515](https://github.com/facebook/jest/pull/10515))
 
 ### Chore & Maintenance
diff --git a/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap b/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap
@@ -0,0 +1,97 @@
+// 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 | });
+      7 |
+      8 | it('test 2', () => {
+
+      at Object.toEqual (__tests__/test-1.js:5:15)
+
+  ● test 2
+
+    expect(received).toEqual(expected) // deep equality
+
+    - Expected  - 1
+    + Received  + 3
+
+    - Object {}
+    + Object {
+    +   "ref": [Circular],
+    + }
+
+      10 |   foo.ref = foo;
+      11 |
+    > 12 |   expect(foo).toEqual({});
+         |               ^
+      13 | });
+
+      at Object.toEqual (__tests__/test-1.js:12: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 | });
+      7 |
+      8 | it('test 2', () => {
+
+      at Object.toEqual (__tests__/test-2.js:5:15)
+
+  ● test 2
+
+    expect(received).toEqual(expected) // deep equality
+
+    - Expected  - 1
+    + Received  + 3
+
+    - Object {}
+    + Object {
+    +   "ref": [Circular],
+    + }
+
+      10 |   foo.ref = foo;
+      11 |
+    > 12 |   expect(foo).toEqual({});
+         |               ^
+      13 | });
+
+      at Object.toEqual (__tests__/test-2.js:12:15)
+`;
+
+exports[`handles circular inequality properly 2`] = `
+Test Suites: 2 failed, 2 total
+Tests:       4 failed, 4 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites.
+`;
diff --git a/e2e/__tests__/circularInequality.test.ts b/e2e/__tests__/circularInequality.test.ts
@@ -0,0 +1,67 @@
+/**
+ * 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({});
+    });
+
+    it('test 2', () => {
+      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}) => {
+    return stderr.includes('Ran all test suites.');
+  });
+
+  const {stderr} = await end();
+
+  const {summary, rest} = extractSortedSummary(stderr);
+  expect(wrap(rest)).toMatchSnapshot();
+  expect(wrap(summary)).toMatchSnapshot();
+});
diff --git a/packages/jest-worker/package.json b/packages/jest-worker/package.json
@@ -16,7 +16,8 @@
   "dependencies": {
     "@types/node": "*",
     "merge-stream": "^2.0.0",
-    "supports-color": "^8.0.0"
+    "supports-color": "^8.0.0",
+    "telejson": "^5.1.0"
   },
   "devDependencies": {
     "@types/merge-stream": "^1.1.2",
diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts
@@ -23,6 +23,7 @@ import {
   WorkerInterface,
   WorkerOptions,
 } from '../types';
+import {parse} from './utils';
 
 const SIGNAL_BASE_EXIT_CODE = 128;
 const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9;
@@ -162,7 +163,7 @@ export default class ChildProcessWorker implements WorkerInterface {
 
     switch (response[0]) {
       case PARENT_MESSAGE_OK:
-        this._onProcessEnd(null, response[1]);
+        this._onProcessEnd(null, parse(response[1]));
         break;
 
       case PARENT_MESSAGE_CLIENT_ERROR:
@@ -195,7 +196,7 @@ export default class ChildProcessWorker implements WorkerInterface {
         this._onProcessEnd(error, null);
         break;
       case PARENT_MESSAGE_CUSTOM:
-        this._onCustomMessage(response[1]);
+        this._onCustomMessage(parse(response[1]));
         break;
       default:
         throw new TypeError('Unexpected response from worker: ' + response[0]);
diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts
@@ -23,6 +23,7 @@ import {
   WorkerInterface,
   WorkerOptions,
 } from '../types';
+import {parse} from './utils';
 
 export default class ExperimentalWorker implements WorkerInterface {
   private _worker!: Worker;
@@ -141,7 +142,7 @@ export default class ExperimentalWorker implements WorkerInterface {
 
     switch (response[0]) {
       case PARENT_MESSAGE_OK:
-        this._onProcessEnd(null, response[1]);
+        this._onProcessEnd(null, parse(response[1]));
         break;
 
       case PARENT_MESSAGE_CLIENT_ERROR:
@@ -175,7 +176,7 @@ export default class ExperimentalWorker implements WorkerInterface {
         this._onProcessEnd(error, null);
         break;
       case PARENT_MESSAGE_CUSTOM:
-        this._onCustomMessage(response[1]);
+        this._onCustomMessage(parse(response[1]));
         break;
       default:
         throw new TypeError('Unexpected response from worker: ' + response[0]);
diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts
@@ -6,6 +6,7 @@
  */
 
 import {PARENT_MESSAGE_CUSTOM} from '../types';
+import {serialize} from './utils';
 
 const isWorkerThread: boolean = (() => {
   try {
@@ -30,9 +31,9 @@ export default function messageParent(
       parentPort,
     } = require('worker_threads') as typeof import('worker_threads');
     // ! is safe due to `null` check in `isWorkerThread`
-    parentPort!.postMessage([PARENT_MESSAGE_CUSTOM, message]);
+    parentPort!.postMessage([PARENT_MESSAGE_CUSTOM, serialize(message)]);
   } else if (typeof parentProcess.send === 'function') {
-    parentProcess.send([PARENT_MESSAGE_CUSTOM, message]);
+    parentProcess.send([PARENT_MESSAGE_CUSTOM, serialize(message)]);
   } else {
     throw new Error('"messageParent" can only be used inside a worker');
   }
diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts
@@ -16,6 +16,7 @@ import {
   PARENT_MESSAGE_OK,
   PARENT_MESSAGE_SETUP_ERROR,
 } from '../types';
+import {serialize} from './utils';
 
 let file: string | null = null;
 let setupArgs: Array<unknown> = [];
@@ -64,7 +65,7 @@ function reportSuccess(result: unknown) {
     throw new Error('Child can only be used on a forked process');
   }
 
-  process.send([PARENT_MESSAGE_OK, result]);
+  process.send([PARENT_MESSAGE_OK, serialize(result)]);
 }
 
 function reportClientError(error: Error) {
@@ -86,7 +87,7 @@ function reportError(error: Error, type: PARENT_MESSAGE_ERROR) {
 
   process.send([
     type,
-    error.constructor && error.constructor.name,
+    error.constructor?.name,
     error.message,
     error.stack,
     typeof error === 'object' ? {...error} : error,
diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts
@@ -17,6 +17,7 @@ import {
   PARENT_MESSAGE_OK,
   PARENT_MESSAGE_SETUP_ERROR,
 } from '../types';
+import {serialize} from './utils';
 
 let file: string | null = null;
 let setupArgs: Array<unknown> = [];
@@ -65,7 +66,7 @@ function reportSuccess(result: unknown) {
     throw new Error('Child can only be used on a forked process');
   }
 
-  parentPort!.postMessage([PARENT_MESSAGE_OK, result]);
+  parentPort!.postMessage([PARENT_MESSAGE_OK, serialize(result)]);
 }
 
 function reportClientError(error: Error) {
diff --git a/packages/jest-worker/src/workers/utils.ts b/packages/jest-worker/src/workers/utils.ts
@@ -0,0 +1,34 @@
+/**
+ * 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 {parse as parseMessage, stringify} from 'telejson';
+
+const customMessageStarter = 'jest string - ';
+
+export function serialize(data: unknown): unknown {
+  if (data != null && typeof data === 'object') {
+    // we only use stringify for the circular objects, not other features
+    return `${customMessageStarter}${stringify(data, {
+      allowClass: false,
+      allowDate: false,
+      allowFunction: false,
+      allowRegExp: false,
+      allowSymbol: false,
+      allowUndefined: false,
+    })}`;
+  }
+
+  return data;
+}
+
+export function parse(data: unknown): unknown {
+  if (typeof data === 'string' && data.startsWith(customMessageStarter)) {
+    return parseMessage(data.slice(customMessageStarter.length));
+  }
+
+  return data;
+}
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ import {`
`17`	`17`	`PARENT_MESSAGE_OK,`
`18`	`18`	`PARENT_MESSAGE_SETUP_ERROR,`
`19`	`19`	`} from '../types';`
	`20`	`+import {serialize} from './utils';`
`20`	`21`
`21`	`22`	`let file: string \| null = null;`
`22`	`23`	`let setupArgs: Array<unknown> = [];`
`@@ -65,7 +66,7 @@ function reportSuccess(result: unknown) {`
`65`	`66`	`throw new Error('Child can only be used on a forked process');`
`66`	`67`	`}`
`67`	`68`
`68`		`- parentPort!.postMessage([PARENT_MESSAGE_OK, result]);`
	`69`	`+ parentPort!.postMessage([PARENT_MESSAGE_OK, serialize(result)]);`
`69`	`70`	`}`
`70`	`71`
`71`	`72`	`function reportClientError(error: Error) {`