jestjs / jest

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

[Bug]: jest-circus runner ignores `toJSON` serialization, causing `Converting circular structure to JSON` warnings #11958

Open blimmer opened 3 years ago

blimmer commented 3 years ago

Version

28.1.1

Steps to reproduce

  1. Clone my repo https://github.com/blimmer/jest-issue-repro
  2. Run npm ci
  3. Run npm run test:jest-jasmine2. You'll see that the error message from the test is properly serialized. You can see that output in GitHub actions without cloning the repo.
 FAIL  ./circular-structure.test.js
  ● circular structure › fails

    Error with thing!

      47 |     // This error should not have a circular reference because `toJSON` is implemented on `thing` that creates a
      48 |     // safe, non-circular object.
    > 49 |     throw new ThingError(thing);
         |           ^
      50 |   })
      51 | })
      52 |

      at Object.<anonymous> (circular-structure.test.js:49:11)
  1. Run npm run test:jest-circus. You'll see an error message about Converting circular structure to JSON. You can see that output in GitHub actions without cloning the repo.
(node:10437) UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'thingManager' -> object with constructor 'Object'
    |     property 'things' -> object with constructor 'Array'
    --- index 0 closes the circle
    at stringify (<anonymous>)
    at writeChannelMessage (internal/child_process/serialization.js:127:20)
    at process.target._send (internal/child_process.js:812:17)
    at process.target.send (internal/child_process.js:710:19)
    at reportSuccess (/Users/blimmer/code/jest-issue-repro/node_modules/jest-worker/build/workers/processChild.js:59:11)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:10437) 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(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:10437) [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.

 RUNS  ./circular-structure.test.js

In my test, I have three classes: Thing, ThingManager (which tracks all Things) and a custom ThingError that has a reference to a thing: https://github.com/blimmer/jest-issue-repro/blob/0ccc0debd32599c2415f9e9dd1c727aa4b1c075c/circular-structure.test.js#L1-L40

The circular reference issue is because each Thing has a reference to itself via ThingManager.things. In the standard Node world, I work around this by defining a toJSON method on Thing that removes the circular reference: https://github.com/blimmer/jest-issue-repro/blob/0ccc0debd32599c2415f9e9dd1c727aa4b1c075c/circular-structure.test.js#L24-L26

Expected behavior

I expected jest-circus to serialize the error message (by calling toJSON) just like jest-jasmine2 did.

Actual behavior

When running with jest-circus, I get an error message about the JSON serialization failing, not the error message from the test itself. This makes it difficult to debug which test is failing, as there's no reference to the issue in the output.

I think I tracked the issue down to this code in jest-circus: https://github.com/facebook/jest/blob/7dd17d541bcdb4d17d96b53586949fb195294040/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts#L96-L99

Because keepPrototype is set to false, I believe it's discarding the toJSON method, which triggers the issue. The code comment makes it seem like this might be a known issue that you're already looking to correct.

Additional context

Interestingly enough, this repro case also exposes the issue here described in https://github.com/facebook/jest/issues/10577 .

If you look at this Github Actions run, you'll notice that the build hangs (and is killed by my 1-minute timeout on the action) on Node 12.x and 14.x.

Screen Shot 2021-10-13 at 4 58 10 PM

Environment

  System:
    OS: macOS 12.4
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 16.15.0 - ~/.asdf/installs/nodejs/16.15.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.5.5 - ~/.asdf/plugins/nodejs/shims/npm
  npmPackages:
    jest: ^28.1.1 => 28.1.1
npwork commented 2 years ago

Happens with axios as well (when axios throws error)

node:internal/child_process/serialization:127
    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:127:20)
    at process.target._send (node:internal/child_process:839:17)
    at process.target.send (node:internal/child_process:739:19)
    at reportSuccess (..../node_modules/jest-worker/build/workers/processChild.js:59:11)

Workaround: 1) add to jest.config

setupFilesAfterEnv: [ './jest.axios-workaround.setup.ts'],

2) file jest.axios-workaround.setup.ts

import axios from 'axios'

axios.interceptors.response.use(
  (response) => response,
  (error) => {
    if ('request' in error) delete (error as any).request
    if ('response' in error) delete (error as any).response?.request
    return Promise.reject(error)
  },
)

(or if you use js, simply remove types)

npwork commented 2 years ago

If you still need internals of request you can use this approach

function axiosErrorWorkaround(error: AxiosError) {
  if (!error?.request) return
  const picked = _.pick(error.request, [
    'outputData',
    'writable',
    'destroyed',
    'chunkedEncoding',
    'shouldKeepAlive',
    'maxRequestsOnConnectionReached',
    'useChunkedEncodingByDefault',
    'sendDate',
    'finished',
    'method',
    'maxHeaderSize',
    'path',
    'aborted',
    'timeoutCb',
    'upgradeOrConnect',
    'host',
    'config',
    '_header',
    '_keepAliveTimeout',
  ])
  error.request = picked
  if (error.response?.request) {
    error.response.request = picked
  }
}
stephenh commented 1 year ago

For others running in to this, @brudil found that Jest is not respecting the prototype when doing the cross-worker serialization, so if you've implemented toJSON as a class method, it will get ignored.

If you implement toJSON as a method directly on the instance, then it'll work fine.

kasir-barati commented 7 months ago

@npwork your first solution did not work for me (it caused my Axios aggregator to throw an error):

Auth › should throw 400 error when lastName is not a valid string; undefined

    AggregateError:

      at Function.Object.<anonymous>.AxiosError.from (../../node_modules/axios/lib/core/AxiosError.js:89:14)
      at RedirectableRequest.handleRequestError (../../node_modules/axios/lib/adapters/http.js:610:25)
      at ClientRequest.eventHandlers.<computed> (../../node_modules/follow-redirects/index.js:38:24)
      at Axios.request (../../node_modules/axios/lib/core/Axios.js:45:41)

    Cause:
    AggregateError:

And your second solution fixes that issue but produces a new one :smiling_face_with_tear::

TypeError: Cannot read properties of undefined (reading 'status')

      25 |       );
      26 |
    > 27 |       expect(res.status).toBe(200);
         |                  ^
      28 |       expect(res.data).toStrictEqual({
      29 |         some: 123,
      30 |       });

      at src/auth/auth-validation.spec.ts:27:18
      at fulfilled (../../node_modules/tslib/tslib.js:166:62)

Here is how I did it:

import axios from 'axios';
import _ from 'lodash';

axios.interceptors.response.use(
  (response) => response,
  (error) => {
    if (!error?.request) {
      return;
    }
    const picked = _.pick(error.request, [
      'outputData',
      'writable',
      'destroyed',
      'chunkedEncoding',
      'shouldKeepAlive',
      'maxRequestsOnConnectionReached',
      'useChunkedEncodingByDefault',
      'sendDate',
      'finished',
      'method',
      'maxHeaderSize',
      'path',
      'aborted',
      'timeoutCb',
      'upgradeOrConnect',
      'host',
      'config',
      '_header',
      '_keepAliveTimeout',
    ]);
    error.request = picked;
    if (error.response?.request) {
      error.response.request = picked;
    }
  },
);

What helped me to figure out what is wrong with my axios req.

At least I realized that it had nothing to do with axios and jest :sweat_smile:, so I added the setupFileAfterEnv so to understand what is going on and it helped me:

import axios from 'axios';

axios.interceptors.response.use(
  (response) => response,
  (error) => {
    console.dir(error, { depth: null });
    return Promise.reject(error);
  },
);