jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.32k stars 6.47k forks source link

Circular references hang jest when assertions fail on node 14 #10577

Closed voces closed 4 months ago

voces commented 4 years ago

πŸ› 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

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, landed in 26.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 
Lonli-Lokli commented 4 years ago

Is there any workaround?

joelcoxokc commented 4 years ago

@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.

ziacik commented 4 years ago

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

rimunroe commented 4 years ago

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:

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.

ziacik commented 4 years ago

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.

ziacik commented 4 years ago

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.

ziacik commented 4 years ago

Added a PR as an attempt to fix this.

Also, please note that the messageParent error mentioned above is due to this line which swallows the real error message which is also about circular references.

SimenB commented 4 years ago

Added a PR as an attempt to fix this.

πŸŽ‰

Also, please note that the messageParent error mentioned above is due to this line which swallows the real error message which is also about circular references.

We should not swallow errors like that...

piotrl commented 4 years ago

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).

daton89 commented 3 years ago

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?

    //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?

Lonli-Lokli commented 3 years ago

@SimenB from angular issue "jest": "26.6.3", "jest-canvas-mock": "2.3.0", "jest-preset-angular": "8.3.2", Now, the correct error is displayed in the console. However, the test runner process still does not stop. i.e. Process does not end when error is found.

maxfriedmann commented 3 years ago

@Lonli-Lokli I have the exact same versions as you, still getting meaningless errors:

(node:6657) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'element' -> object with constructor 'Object'
    |     property 'componentProvider' -> object with constructor 'Object'
    --- property 'parent' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (internal/child_process/serialization.js:117:20)
    at process.target._send (internal/child_process.js:805:17)
    at process.target.send (internal/child_process.js:703:19)
    at reportSuccess (/builds/smallstack/products/cloud/node_modules/jest-runner/node_modules/jest-worker/build/workers/processChild.js:67:11)
(Use `node --trace-warnings ...` to show where the warning was created)

The error should have been a NullInjectorError: NullInjectorError: No provider for CloudApiClient

tonivj5 commented 3 years ago

I have debugged a bit why this happens, and it happens because Angular TestBed configure its compiler to add some additional meta-info to the errors. That configuration is getted from the same file, calling to initServicesIfNeeded function from some place into TestBed.

I don't know if it should be fixed by jest or Angular (maybe implementing a toJSON method to its errors, to avoid to include this meta-properties to serialization). But it's a problem with jest-workers and Angular TestBed errors

CSchulz commented 3 years ago

Why not just use https://github.com/WebReflection/circular-json or https://www.npmjs.com/package/json-stringify-safe or similar libraries to be safe with Object JSON stringify?

SimenB commented 3 years ago

The fix for this will be reverted (#11172) as it caused other serialization issues. Might need to use a custom json serializer, yeah

robertleeplummerjr commented 3 years ago

What is the recommended way of handling this?

samsieber commented 3 years ago

Personally, I recommend rolling back the jest-jasmine2 (or jest-circus) version to one that isn't broken. I haven't tried it for a while yet, so there might be mysterious failures if you use try to use features introduced by the newer jest versions; you can do that if you don't directly depend on jest-jasmine2 by adding this to your package.json:

    "resolutions": {
        "jest-jasmine2": "26.0.1"
    }
robertleeplummerjr commented 3 years ago

In our case this ended up being an uncaught exception. We eventually were able to log out the exception, and fix the unit test. Then it worked fine. Just FYI if anyone else runs into this. Scary stuff.

BingeCode commented 3 years ago

For Angular users this might be the fix/workaround:

  1. Error reads something like

TypeError: Converting circular structure to JSON --> starting at object with constructor 'Object' | property 'element' -> object with constructor 'Object' | property 'componentProvider' -> object with constructor 'Object' --- property 'parent' closes the circle at stringify ()

  1. Run npm run test:detectOpenHandles or the equivalent jest --detectOpenHandles
  2. Error now reads

Found the synthetic listener @transform.start. Please include either "BrowserAnimationsModule" or "NoopAnimationsModule" in your application.

OR

Found the synthetic property @transitionMessages. Please include[...]

  1. To fix, import BrowserAnimationsModule or NoopAnimationsModule in your test

I don't know what the above errors are caused by though. Hopefully this helps.

EDIT: Formatting

blimmer commented 3 years ago

I was creating a repro case for a similar issue (see #11958) and I noticed that the issue of the jest runner hanging only appears to affect jest-circus on Node 12.x and 14.x.

If you look at this GitHub actions run: https://github.com/blimmer/jest-issue-repro/actions/runs/1339553822 you'll see that 12.x and 14.x get killed by a timeout, while 16.x completes normally.

So, for those who are able to upgrade to Node 16.x, that's another possible workaround to this issue.

sparachi commented 3 years ago

I've faced this issue recently in a React-Native project. The was due to declaring ref on a TouchableOpacity element. Which created a huge snapshot I guess, and the expectSnapshot failed. I removed from it and used on a wrapper view to get around this error. Btw, do not forget to use collapsable on the view, else it will cause App crash. Screen Shot 2021-11-16 at 1 54 05 PM

gabsmprocha commented 2 years ago

same here, any solution ?

node: v14.17.6 npm: 6.14.15 jest: 24.8.0

Lonli-Lokli commented 2 years ago

@SimenB I know you might not have time for this work but do you still remember about this issue? ;)

G-Rath commented 2 years ago

@SimenB very tentative looking back into this (I just can't help myself sometimes πŸ™ˆ) and can't comment on my PR as it's now locked.

You outlined what you think the high level solution for this should be here: https://github.com/facebook/jest/pull/11467#issuecomment-1040878924

I'm wondering if you had any thoughts on how to do that? From what I remember when I the failureDetails property it's structure was very ad-hoc (starting with "a bunch of arrays or a single error", iirc) so I'm concerned about how this could actually be solved robustly.

It could be that I'm misremembering or that ultimately the types boil down to a stable union of complex but well defined types. (I want to say the type of that property is unknown but I'm on mobile so can't confirm - if it is I'd say a good first step is probably to get that property's type completely defined)

G-Rath commented 2 years ago

cc @segrey I imagine you're probably pretty familiar with the different values the failureDetails property can be expected to hold - since it's likely that fixing this will result in less details, it would be good if you could detail what information you're actually using in WebStorm so we can keep them.

maple-leaf commented 2 years ago

I think we can fix this by patching the jest-worker, when detecting a circular error, just fail the case.

diff --git a/node_modules/jest-worker/build/workers/processChild.js b/node_modules/jest-worker/build/workers/processChild.js
index a4f5acb..4c2a018 100644
--- a/node_modules/jest-worker/build/workers/processChild.js
+++ b/node_modules/jest-worker/build/workers/processChild.js
@@ -64,7 +64,13 @@ function reportSuccess(result) {
     throw new Error('Child can only be used on a forked process');
   }

-  process.send([_types().PARENT_MESSAGE_OK, result]);
+  try {
+    process.send([_types().PARENT_MESSAGE_OK, result]);
+  } catch (e) {
+    if (e.message.indexOf("circular") > -1) {
+      reportClientError(e);
+    }
+  }
 }

 function reportClientError(error) {
vitalyster commented 2 years ago

Same issue on node 18

G-Rath commented 2 years ago

@SimenB I've dug into this and confirmed that if I comment out this line the issue goes away, and also that this is because test.errors holds both Jest errors and user errors.

So to ensure that that is JSON serializable I think we've got three options:

  1. Only hold Jest-based errors (e.g. AssertionResult), which internally would be ensured are JSON serializable
    • From what I can tell AssertionResult is only represented as a type so we'd have to come up with a stable way to check if an error is of that type (which could be as simple as matcherResult in error, but it'd have to be stable)
    • I don't know if there are other Jest errors that end up here that we'd also want to preserve
    • This would mean that reporters could not have any special handling on non-jest errors (e.g. pretty handling of ESLint errors)
  2. Use a library like flatted to serialize the content
    • My understanding is there is not a single standard for serializing circular json, so this would mean consumers would be required to use whatever Jest decides to use, which could be an annoying overhead
    • This'd mean any tools currently using this property would require updating since they'll currently be expecting the content as a standard object rather than a string that needs to be parsed by an external library
  3. Require downstream users to ensure their errors are serializable as JSON
    • This would have huge overhead and based on #11958 there might even be conflicts in Jest that makes it impossible for users to do this

I think 1. is probably the option to go with at least for - afaik JetBrains are actually the only people using this property and they're definitely only interested in the AssertionResult errors from Jest; I think that should also be easier to extend afterwards if someone did want to preserve user errors e.g. we could implement a config option for providing a handler to do the serialization and only apply that to non AssertionResult errors.

G-Rath commented 2 years ago

After digging further into this I think I've come to what you were outlining here @SimenB, which is that AssertionError is not being properly serialized.

The bigger problem with this is that it's the actual and expected properties that are not being properly serialized which feels to me like it almost puts us back to where we started with #11467 just higher up πŸ€” I've not looked into how reporters actually fit into Jest, but I'm wondering if maybe the best solution here would be to accept a parse/stringify pair that gets called here + before the final data gets passed to whatever reporters are being used - that way people can opt-in to handling this how they want and hopefully we don't have to do a breaking change for all existing reporters...

That'd also fix the issue for both AssertionError & external errors. @SimenB what do you think?

KrisBNelson commented 2 years ago

I get this issue with mocking await functions in many tests in different describes in the same file which all fail when I have a mockReturnValueOnce in one describe, but doesn't appear with mockReturnValue in the describe block, it's as if the mocks are shared between describe blocks because the return value from an upper describe was being returned in my other describe block mock instead of its own return value...

node:internal/child_process/serialization:127
    const string = JSONStringify(message) + '\n';
                   ^

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property '_httpMessage' -> object with constructor 'Object'
    --- property 'socket' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (node:internal/child_process/serialization:127:20)
    at process.target._send (node:internal/child_process:819:17)
    at process.target.send (node:internal/child_process:719:19)
    at reportSuccess (C:\node-projects\telus\tps\cdo-tps-quote-mgmt-api\node_modules\jest-worker\build\workers\processChild.js:59:11)

If I do jest --detectOpenHandles or jest --runInBand the errors don't appear, and I don't have a bunch of tests fail anymore.

gaurav5430 commented 2 years ago

@KrisBNelson yeah, the circular reference error does not happen with jest --runInBand , why ?

stephenh commented 2 years ago

@gaurav5430 because with --runInBand everything (assertion failures/etc.) stays within a single Node process.

It's only when Jest has N child worker processes that, when a test fails in the child worker, the assertion failure needs to be serialized (via JSON) and sent to the parent process, and then a "cannot convert to JSON" error happens.

gaurav5430 commented 2 years ago

@gaurav5430 because with --runInBand everything (assertion failures/etc.) stays within a single Node process.

It's only when Jest has N child worker processes that, when a test fails in the child worker, the assertion failure needs to be serialized (via JSON) and sent to the parent process, and then a "cannot convert to JSON" error happens.

what's the actual solution for this issue? would downgrading jest work ?

simon-chen-1 commented 2 years ago

Any solution for this issue?

G-Rath commented 2 years ago

@SimenB gentle nudge to get your thoughts on what I've outlined in this and that comment :)

yeahio commented 1 year ago

@SimenB Kindly have this bug fixed, please!

prapurnakumariTR commented 1 year ago

hey, any solution on this issue ?

theKashey commented 1 year ago

Similar problem - expect(someDomeNode).toBe(someOtherNode) loops somewhere in the serialization of endless React/JSDOM structure and ends as an OOM exception.

adamflush commented 1 year ago

Node v16.19 Jest 29.5 simply calling a method that raises an error with a circular reference.

Try/catching the method call and replacing the error in the catch is a work-around. Without that, jest throws the unintuitive error message. Example:

       TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'ClientRequest'
        |     property 'socket' -> object with constructor 'TLSSocket'
        --- property '_httpMessage' closes the circle
        at stringify (<anonymous>)

      at messageParent (node_modules/jest-worker/build/workers/messageParent.js:29:19)

The underlying dependency of the package throwing the error in this case is Axios.

The root problem with Jest is assuming JSON.stringify is always going to work. Use a safe stringify implementation, please.

mrazauskas commented 1 year ago

Currently a solution could be workerThreads: true option.

Note that it is marked experimental. That is recently added feature and it is not tested widely yet.

obalilty commented 1 year ago

i was getting the same error for ZoneJs object with circular structure was able to workaround this issue by adding to following to testWorker.js (line 127)

const sendMessageToJest = (eventName, args) => {
  // start here
  if (args[1].failureDetails) {
    args[1].failureDetails.forEach((fd) => {
      fd.rejection = null;
      fd.promise = null;
      fd.zone = null;
      fd.task = null;
    })
  };
  // end here
  (0, _jestWorker().messageParent)([eventName, args]);
};

now getting the real error with stacktrace message and without any "Converting circular structure to JSON" errors you may need to make adjustments depending on your code

A more correct approach might be to set null for each property of failureDetails item except 'stack' and 'message' props

jest v28.1.0 or 29.5.0 node v16.19.0

tim-sh commented 1 year ago

With jest@29.5.0, the following generic workaround helped me:

function deCircle(o, seen = new Set()) {
  if (!o || typeof o !== 'object') return;
  Object.entries(o).forEach(([k,v]) => {
    if (seen.has(v)) {
      o[k] = null;
      return;
    }
    const s = new Set(seen);
    s.add(v);
    deCircle(v, s);
  })
}
function reportSuccess(result) {
  if (!process || !process.send) {
    throw new Error('Child can only be used on a forked process');
  }
  deCircle(result); // ← patching the object
  process.send([_types.PARENT_MESSAGE_OK, result]);
}

You might need to patch on error as well, depending on the circumstances.

felipebutcher commented 1 year ago

I had this problem a few minutes ago. Found out it happened when I had two test files with same name in different folders. Once I renamed one of the files the problem was gone.

danielo515 commented 1 year ago

Glad to see this is not a general problem with testing eslint rules. In my case, when a test fails, because almost all eslint nodes have circular references this problem is quite common.

kasir-barati commented 10 months ago

Still happening even though I am just expecting a number to be there and even thought it is but still tests fails 😭 Look here: https://github.com/kasir-barati/nestjs-materials/blob/main/typeorm/src/modules/talent-e2e/get-all.e2e-spec.ts#L9

The Solution that worked for me

https://github.com/jestjs/jest/issues/10577#issuecomment-1262528952

kasir-barati commented 8 months ago

Coming back again to the same issue, it seems that this time adding --detectOpenHandles flag is not gonna help me, I cannot deduce the issue from the logs and when I add that flag test suit fails immediately after it reaches the point that it should make a http req via axios (so no room to log anything in my test suit) and just logs that axios request failed with 400 http status code but in reality it even won't enter the catch block, any idea?

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'res' -> object with constructor 'Object'
    --- property 'req' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (node:internal/child_process/serialization:159:20)
    at process.target._send (node:internal/child_process:852:17)
    at process.target.send (node:internal/child_process:752:19)
    at reportSuccess (/home/kasir/projects/you-say/node_modules/jest-worker/build/workers/processChild.js:82:11)

Node.js v20.10.0
 FAIL   backend-e2e  apps/backend-e2e/src/auth/auth-business.spec.ts
  ● Test suite failed to run

    Jest worker encountered 4 child process exceptions, exceeding retry limit

      at ChildProcessWorker.initialize (../../node_modules/jest-worker/build/workers/ChildProcessWorker.js:181:21)
sawvox commented 8 months ago

Coming back again to the same issue, it seems that this time adding --detectOpenHandles flag is not gonna help me, I cannot deduce the issue from the logs and when I add that flag test suit fails immediately after it reaches the point that it should make a http req via axios (so no room to log anything in my test suit) and just logs that axios request failed with 400 http status code but in reality it even won't enter the catch block, any idea?

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'res' -> object with constructor 'Object'
    --- property 'req' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (node:internal/child_process/serialization:159:20)
    at process.target._send (node:internal/child_process:852:17)
    at process.target.send (node:internal/child_process:752:19)
    at reportSuccess (/home/kasir/projects/you-say/node_modules/jest-worker/build/workers/processChild.js:82:11)

Node.js v20.10.0
 FAIL   backend-e2e  apps/backend-e2e/src/auth/auth-business.spec.ts
  ● Test suite failed to run

    Jest worker encountered 4 child process exceptions, exceeding retry limit

      at ChildProcessWorker.initialize (../../node_modules/jest-worker/build/workers/ChildProcessWorker.js:181:21)

Trying to stringify the req object from Axios will result in the circular reference: req -> res -> req As your error states:

--> starting at object with constructor 'Object' | property 'res' -> object with constructor 'Object' --- property 'req' closes the circle

Try comparing the properties of the request or response object that you care about rather than the whole object itself.

I.e. expect(res.body?.myProperty).toEqual("foo")

kasir-barati commented 8 months ago

So @sawvox what you're saying is that if I just compare what I wanted in the request or response object I would not face this issue but I guess that is not something to compromise, But thanks for uncovering one more mystery for me. I did not know that it is happening because of my expects.

franzos commented 7 months ago

Today I've started seeing similar errors on a repository that I've been working on for 4 years. Funny thing is, it's really random - sometimes it works, sometimes it doesn't. Flags like --runInBand seem the increase the likelihood that it works, but it's no guarantee.

node:internal/child_process/serialization:159
    const string = JSONStringify(message) + '\n';
                   ^

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'socket' -> object with constructor 'Object'
    --- property '_httpMessage' closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (node:internal/child_process/serialization:159:20)
    at process.target._send (node:internal/child_process:838:17)
    at process.target.send (node:internal/child_process:738:19)
    at reportSuccess (/usr/src/app/node_modules/.pnpm/jest-worker@29.7.0/node_modules/jest-worker/build/workers/processChild.js:82:11)

Node.js v18.16.1

Also tried the suggested workerThreads: true but this will lead to a different error:

    DataCloneError: function transformRequest(data, headers) {
        const contentType = headers.getContentType() || '';
        const ha...<omitted>... } could not be cloned.

      at messageParent (../../node_modules/.pnpm/jest-worker@29.7.0/node_modules/jest-worker/build/workers/messageParent.js:24:34)

This is on Jest 29.5.12.

steebchen commented 5 months ago

Also running into this, the worst thing is that the stacktrace only shows files in node_modules, which makes it extremely hard to debug where the error exactly came from in your tests. Running into this with node@v20.10.0 and jest@29.7.0