jest icon indicating copy to clipboard operation
jest copied to clipboard

jest-worker fails with "Do not know how to serialize a BigInt" in `messageParent.ts` instead of showing the actual assertion error message when there are 2 or more test suite files executing in parallel

Open klesun opened this issue 3 years ago • 9 comments

Link to repl or repo (highly encouraged)

Minimal reproduction repo: jest-bigint-worker-issue

🐛 Bug Report

When you have multiple test suite files executing in parallel, and some of the tests are asserting values with bigint numbers in them and the assertion fails, instead of showing this assertion error message, jest-worker fails itself on attempt to serialise this value during the call to the:

parentProcess.send([_types().PARENT_MESSAGE_CUSTOM, message]);

(in messageParent.js)

I assume that's because message includes failureDetails.matcherResult holding the compared bigint values that can't be passed to process.send(), as unlike worker_threads it does not support non-json values out of the box.

To Reproduce

Steps to reproduce the behavior:

git clone [email protected]:klesun-productions/jest-bigint-worker-issue.git # clone minimal reproduction repo
cd jest-bigint-worker-issue
npm ci # install jest dependency
npm test # run the test that reproduces the issue

Expected behavior

You should have seen the assertion error informing you that expect(1n).toEqual(2n); expectation failed

Actual behavior

But instead you get following output due to an internal jest-worker error:

 PASS  tests/some-other.test.js
  ✓ should succeed (2 ms)

 FAIL  tests/bigint.test.js
  ● Test suite failed to run

    TypeError: Do not know how to serialize a BigInt
        at stringify (<anonymous>)

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

Test Suites: 1 failed, 1 passed, 2 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.777 s

With no details of what the actual error that jest-worker tried to report was in tests/bigint.test.js.

envinfo

  System:
    OS: macOS 11.3.1
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 14.17.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.13 - /usr/local/bin/npm
  npmPackages:
    jest: ^27.0.5 => 27.0.5

Misc

I assume that's a transport issue that user of jest is not supposed to be aware of. One thing I did that at least somehow solved the absence of the real message was patching the jest-worker wrapping the call to parentProcess.send() in a try-catch and console.log-ing the message whose serialisation failed:

try {
  parentProcess.send([_types().PARENT_MESSAGE_CUSTOM, message]);
} catch (error) {
  console.error('jest-worker message serialisation failed', error);
  console.dir(message, {depth: 10});
  throw error;
}

There is likely a number of ways how to better address this problem...

klesun avatar Jun 28 '21 15:06 klesun

@SimenB, since you changed this part of jest-worker not so long ago, maybe you could, please, give us an advice here for the discussion to move in the right direction?

klesun avatar Jun 28 '21 16:06 klesun

Related: https://github.com/facebook/jest/pull/10293

@kunal-kushwaha fyi

klesun avatar Jun 28 '21 17:06 klesun

cc @sauravhiremath

kunal-kushwaha avatar Jun 28 '21 17:06 kunal-kushwaha

Will look into this tomorrow!

sauravhiremath avatar Jun 28 '21 18:06 sauravhiremath

The choice to use JSON stock JSON serialization moving forward is not helpful. While Jest has been doing better and better with comparison support of native BigInt, the reporting of assertion failures (for me, OOB) still consistently reports an inability to serialize the bigint. With this configuration (OOB) of Jest, polyfills to fix this problem are inconsistent at best.

There are new serializers that can be configured and dropped in for the one way (or even two-way if you configure them correctly) representations for error reports, for example: json-bigint

SHaTRO avatar Sep 05 '21 20:09 SHaTRO

For those of you that got here because a failing test involves some BigInt, you can temporarily circumvent this issue by setting maxWorkers: 1 in your jest configuration.

gomain avatar Feb 03 '22 06:02 gomain

For those of you that got here because a failing test involves some BigInt, you can temporarily circumvent this issue by setting maxWorkers: 1 in your jest configuration.

@gomain That actually works, but I cannot understand why turning down the maxworkers do the trick. Have you found any other solution that does not have impact on the total test time?

JCastrejonE avatar Mar 14 '22 22:03 JCastrejonE

@JCastrejonE serialization (currently using JSON, it does not support BigInt) only happens when sending data between worker threads. Limiting workers to 1 eliminates the need to serialize at all. Until a fix for this, we wait for our single worker to finish.

If only a small number of tests involve BigInt you could separate them, such as (!!! not tested !!!)

{
  "scripts": {
    "test:no-bigint": "jest --testPathIgnorePatterns=.*\\.bigint\\.test\\.js",
    "test:bigint": "jest --maxWorkers=1 --testPathPattern=.*\\.bigint\\.test\\.js",
    "test": "npm run test:no-bigint && npm run test:bigint"
  }
}

gomain avatar Mar 16 '22 04:03 gomain

Any update?

spreadred avatar Aug 04 '22 20:08 spreadred

This is quite unplesant issue. Testing library cannot handle one of built-in language types? Srsly? Please fix it somebody!

Bast1onCZ avatar Sep 23 '22 18:09 Bast1onCZ

+1

bmeeder22 avatar Oct 17 '22 16:10 bmeeder22

+1

dexterastin avatar Oct 18 '22 06:10 dexterastin

+1

n1kk avatar Nov 28 '22 13:11 n1kk

Developing a cryptography package now. Looking forward to any fix to this!

SiegeSailor avatar Dec 17 '22 00:12 SiegeSailor

+1

bmeeder22 avatar Feb 01 '23 15:02 bmeeder22

+1

bam4564 avatar Feb 17 '23 03:02 bam4564

Workaround I put into my project as a post install hook:

import { readFile, writeFile } from 'fs/promises'; import { resolve } from 'path';

// https://github.com/facebook/jest/issues/11617
// Fix for Jest failing to serializbe BigInt.
// Is unsafe for values above Number.MAX_SAFE_INTEGER
export async function hook() {
  async function patch(file) {
    await readFile(file, 'utf-8').then((content) => {
      if (content.includes('// Allow unsafe bigint serialization')) return;

      content =
        [
          '// Allow unsafe bigint serialization',
          "BigInt.prototype['toJSON'] = function () {",
          '  return Number(this);',
          '};',
        ].join('\n') +
        '\n' +
        content;

      return writeFile(file, content).then(() =>
        console.log('Patched jest-worker'),
      );
    });
  }

  const expectedFilePos = resolve(
    process.cwd(),
    `./node_modules/.pnpm/[email protected]/node_modules/jest-worker/build/workers/messageParent.js`,
  );

  // Automatically resolving the path worked, but the resolved module was not the one actually being used by jest.
  // Might be because I'm using pnpm or because jest-worker is a subdependency... IDK
  //
  // const a = require.resolve('jest-worker');
  // const dir = dirname(a);
  // const expectedFilePos = './workers/messageParent.js';
  // const file = resolve(dir, expectedFilePos);
  return patch(expectedFilePos);
}

This will allow jest to keep running. However this might cause issues as the serialized bigint can be incorrect.

Aderinom avatar Feb 22 '23 09:02 Aderinom

Anu updates on solving this issues on jest without workarounds?

arkh-consensys avatar Mar 07 '23 13:03 arkh-consensys

The workerthreads configuration option might help. It is a new feature which was shipped yesterday with Jest v29.5.

Note that this is !!! strictly experimental !!! and wasn’t tryout widely at all. Use it at your own risk.

Currently I run tests with workerThreads: true in one repo. Give it a try.

mrazauskas avatar Mar 07 '23 13:03 mrazauskas

Realized Prisma is returning BigInt which made my zod parsing fail on a raw query while validating the return data, which lead me to here. Luckily our system is running the latest Node which will support structured cloning and we're up to date to be on a version of Jest that supports the experimental workerThreads, but I would prefer to get away from them, as it seems I do not get a performance boost from the flag, if anything a slow down. I guess another route would have been to just make the type validation on Zod to be looser to accept int or BigInt, but I'd rather be more explicit here...

ctsstc avatar Oct 17 '23 04:10 ctsstc

I'm currently developing using NestJS, and when I encountered this issue, I looked it up and found this

I was able to solve it from the following site: https://zenn.dev/serinuntius/scraps/02dfe48b643745

jest.config.ts

"workerThreads": true 

I'm not using jest.config.ts, so I just added that code to the jest part of my package.json and that solved it. I'm not sure why this solves it, though, so if anyone knows how, I'd appreciate a reply.

ldhbenecia avatar Dec 04 '23 09:12 ldhbenecia

One thing to note is that your error messages on failing tests when utilizing workerThreads are much more confusing and less helpful. I think if I recall correctly this is due to something like workerThreads utilizing the new structured clone, while the other was likely utilizing JSON serialization. So if you utilize something like Decimal JS it does not support structured cloning which will then have Jest blowing up on its internals 😬 ie: https://github.com/MikeMcl/decimal.js/issues/224 It's good to note this is still an experimental feature, so it may be unstable or change.

ctsstc avatar Dec 07 '23 01:12 ctsstc

Same Prisma issue as @ctsstc. It's funny how passing tests work, but then upon failing assertion it errors. I guess it's the failing test results that causes returning the data and fail serialization. And as background we have added support for bigints on our JSON.parse/stringify but this doesn't seem used by the workers then for some reason.

Does someone have skills to revive @sauravhiremath attempt fixing this https://github.com/jestjs/jest/pull/11624#issue-934013512

leppaott avatar Jan 25 '24 18:01 leppaott

FYI Setting just the maxWorkers: 1 in jest config doesn't work for me. I had to set workerThreads: true too.

// Enable worker threads for assertion failures involving BigInt
// See https://github.com/jestjs/jest/issues/11617#issuecomment-1458155552
workerThreads: true,
maxWorkers: 1,

EDIT: But workerThreads is veeery expiremental, a lot of jest built-in matches work terribly then...

We ended up switching to vitest :( Even though there's a few things I preferred in jest over vitest.

jtomaszewski avatar Mar 28 '24 12:03 jtomaszewski

I just ran into this issue as well. set maxWorkes: 1 works, but of course it runs very slowly when you have a large number of unit tests. edit: upgrading to latest "jest": "^29.7.0", fixes the issue for me. This with default 50% maxWorkers

  // maxWorkers: 1,
  maxWorkers: "50%",
  workerThreads: true,

jkristia avatar Mar 29 '24 15:03 jkristia