Jest: Non-deterministic "Unexpected end of JSON input" from convert-source-map

[AlexLandau]

🐛 Bug Report

Using Jest 24.9.0 and ts-jest 24.0.2, we are getting non-deterministic build failures with this stacktrace:

  ● Test suite failed to run

    SyntaxError: Unexpected end of JSON input
        at JSON.parse (<anonymous>)

      at new Converter (../../node_modules/convert-source-map/index.js:50:48)
      at Object.exports.fromComment (../../node_modules/convert-source-map/index.js:106:10)
      at Object.exports.fromSource (../../node_modules/convert-source-map/index.js:116:22)
      at ScriptTransformer.transformSource (../../node_modules/@jest/transform/build/ScriptTransformer.js:468:61)
      at ScriptTransformer._transformAndBuildScript (../../node_modules/@jest/transform/build/ScriptTransformer.js:524:40)
      at ScriptTransformer.transform (../../node_modules/@jest/transform/build/ScriptTransformer.js:568:25)

The chance of any individual jest run failing this way is small, but due to the large number of projects in our repo, we are seeing it frequently in our CI builds.

The following bit of code is where the error happens:

    if (!transformed.map) {
      //Could be a potential freeze here.
      //See: https://github.com/facebook/jest/pull/5177#discussion_r158883570
      const inlineSourceMap = convertSourceMap.fromSource(transformed.code);

      if (inlineSourceMap) {
        transformed.map = inlineSourceMap.toJSON();
      }
    }

The fromSource call results in an exception being thrown. Based on some added logging, convertSourceMap is trying to convert a comment with some base-64-encoded JSON into that JSON, but the comment is just cut off in the middle, resulting in JSON that fails to parse.

I suspect the root cause here is some concurrency issue resulting in transformed.code containing an incomplete copy of the file that is cut off early. However, I'm interested in whether you'd accept a PR to address this at a shallower level in the meantime by catching exceptions thrown by fromSource and treating them the same as if it had returned undefined or an empty string. The code already handles the case where that happens, and the worst that would happen (AFAICT) is the absence of source mappings rather than an incorrect test failure.

To Reproduce

I don't have a public repro for this issue. Our repo is large and proprietary, and the issue is non-deterministic. However, some aspects we believe are relevant:

• We run Jest with --maxWorkers=4
• We use ts-jest 24.0.2
• The Jest config includes the following possibly-relevant parts:

    transform: {
      "\\.(js|jsx|ts|tsx)$": "ts-jest",
    },
    globals: {
      "ts-jest": {
        tsConfig: "tsconfig.jest.json",
        diagnostics: false,
        isolatedModules: true
      },

• Once Jest has been run and passes, subsequent runs won't hit this issue unless the Jest cache is cleared (not surprising, since the code path from the stacktrace isn't hit if the cache is used)

Expected behavior

envinfo

(Note: This is occurring in a CircleCI container; the results below are from that environment.)

  System:
    OS: Linux 4.4 Ubuntu 18.04.2 LTS (Bionic Beaver)
    CPU: (36) x64 Intel(R) Xeon(R) Platinum 8124M CPU @ 3.00GHz
  Binaries:
    Node: 10.13.0 - /usr/local/bin/node
    Yarn: 1.12.3 - /usr/local/bin/yarn
    npm: 6.4.1 - /usr/local/bin/npm

[thymikee]

PRs welcome!

[shalomsam]

Is this fix published to an alpha or beta version??

[RajaBellebon]

@SimenB when will we be able to get a version with this fix included? Thank you!

[mr-flannery]

Unfortunately, we're experiencing this as well in our test suite. So +1 from me on being interested in having this in a release :) Thanks folks, keep it up!

[AlonMiz]

+1 causing our build to fail 1 in 25 builds. which is quite a lot from a big R&D team perspective.
maybe it worth a minor or a patch... the release gaps are quite big

[RajaBellebon]

@SimenB @thymikee is the fix included in this version 25.1.0? I don't see it mention in the changelogs but maybe I missed it.

[thymikee]

@RajaBellebon we publish from master and this is merged, so yes, it's included. Look for a PR number that fixed it in the changelog: #9058