Pyright: Cannot find module 'worker_threads' in v1.1.46 using GitHub action

[Borda]

Describe the bug
just after today's release

[email protected]
added 1 package from 1 contributor and audited 1 package in 0.982s
found 0 vulnerabilities

all test started to fail with error unrelated to the python code

Run $(npm bin)/pyright --project .pyrightconfig.json
internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module 'worker_threads'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at eval (webpack:///external_%22worker_threads%22?:1:18)
    at Object.worker_threads (/home/runner/work/pytorch-lightning/pytorch-lightning/node_modules/pyright/dist/pyright.js:2380:1)
    at __webpack_require__ (/home/runner/work/pytorch-lightning/pytorch-lightning/node_modules/pyright/dist/pyright.js:20:30)
    at eval (webpack:///./src/analyzer/program.ts?:23:26)
    at Object../src/analyzer/program.ts (/home/runner/work/pytorch-lightning/pytorch-lightning/node_modules/pyright/dist/pyright.js:1453:1)
    at __webpack_require__ (/home/runner/work/pytorch-lightning/pytorch-lightning/node_modules/pyright/dist/pyright.js:20:30)

To Reproduce
https://github.com/PyTorchLightning/pytorch-lightning/pull/2375/checks?check_run_id=811925548

Additional context
Running in Github Action

[erictraut]

Thanks for reporting the problem. To confirm, are you pretty sure you weren't seeing this with the previous version (1.1.45)?

[Borda]

it started this morning which correlates with your release time...
to verify, just freeze version on 1.1.45 and it runs fine :rabbit:

[jakebailey]

That CI pipeline is using Node v10, which only offers worker threads with the --experimental-worker flag. It's available by default in v11+. I think pyright assumes a pretty new version.

I don't think your pipeline use node for anything but pyright; can you upgrade to a newer (supported/LTS) version, like 12 or 14?

[erictraut]

Thanks, that's a useful clue. Apologies for the regression. I'll look into it.

One more piece of information that would be useful for me... what version of node do you have installed on your system? ("node --version")

[jakebailey]

As for why this is happening now, 5b5cd448a57b69c58364238d21f2f4eaeae7288d added an isMainThread check to to the analysis for logging purposes, whereas before the threading checks were only limited to the LS side (where there are two threads).

[erictraut]

Good analysis, Jake. Sounds like our options are:

1. Ask users to update to a newer version of node
2. Eliminate the dependency for the command-line version of pyright by removing the isMainThread check in that code path

@jakebailey, @heejaechang thoughts? I suppose we could pass in the isMainThread parameter to the program and always set it to false for the code path that the CLI uses.

[jakebailey]

The LogTracker could be passed in the constructor, and the foreground/background analysis pass in new LogTracker(console, 'FG') and new LogTracker(console, 'BG') respectively.

But I'll also note that our node types and testing environment are on v14 and v12 respectively; I don't think we are trying to support these older builds. VS Code is v12 so we target that at the top end (which means the node types should be v12 too, but...).

[erictraut]

That's a good point. I think it's reasonable to raise the minimum bar to node v12 for the CLI. I'd want to add a runtime check so pyright provides a better error message rather than crashing. OK, let's go with that solution.

@Borda, does that work for you? Are you able to upgrade your node version to something less ancient?

[jakebailey]

We can set an engine in the CLI package with the supported node versions, which will error out on install if the node version isn't supported: https://docs.npmjs.com/files/package.json#engines

e.g. { "engines" : { "node" : ">=12.0.0" } } (but I haven't tested)

[erictraut]

Good suggestion. I incorporate the "engines" section in package.json and verified that npm emits a warning when installing it on an incompatible node version.

[Borda]

One more piece of information that would be useful for me... what version of node do you have installed on your system? ("node --version")

we are using default config from GH action so not sure how to do it...

[jakebailey]

Change this:

      - name: Set up node
        uses: actions/setup-node@v1

To this:

      - name: Set up node
        uses: actions/setup-node@v1
        with:
          node-version: '12'

(https://github.com/actions/setup-node#usage)

[Borda]

Change this: (https://github.com/actions/setup-node#usage)

it would be nice to add this instruction also to the GH action for pyright
just running the updated version here - https://github.com/PyTorchLightning/pytorch-lightning/runs/812313646

[jakebailey]

Looks like it worked, thanks for checking!

[heejaechang]

let me make "IsMainThread" to be indirect so that people can use it without requiring worker_thread.

[jakebailey]

Since the minimum version was bumped, we can probably skip that for now (other than for code cleanliness reasons).

[Borda]

yeah, feel free to close it... :]

[jakebailey]

Eric typically closes these on release (hence the label to ensure it's in the changelog), since that'll be the place that enforces the version.

[erictraut]

Thew new requirement is in package.json for version 1.1.47, which I just published.