Latest version fails to start plugin with `Cannot read property 'toString' of null`

nickjs commented 3 years ago

Hi Matt et al, thanks for the latest update to address workspace security. Unfortunately since installing 1.3.0, the typescript-tslint-plugin fails to start for all users in our organization's workspace. I'm not sure if the problem lies in typescript-tslint-plugin or on the vscode-typescript-tslint-plugin side, nor how to helpfully debug to get a more accurate picture.

Here is the output from tsserver.log:

Info 41   [17:35:30.47] [typescript-tslint-plugin] "Computing tslint semantic diagnostics for '/home/nsmall/go/src/github.com/...'"
Info 42   [17:35:30.47] [typescript-tslint-plugin] "(runTsLint) start"
Info 43   [17:35:30.47] [typescript-tslint-plugin] "(loadLibrary) trying to load /home/nsmall/go/src/github.com/..."
Info 44   [17:35:30.47] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Begin - Resolve Global Package Manager Path for: npm"
Info 45   [17:35:30.285] [typescript-tslint-plugin] "'npm config get prefix' value is: /home/nsmall/.npm-global"
Info 46   [17:35:30.286] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Done - Resolve Global Package Manager Path for: npm"
Info 47   [17:35:30.439] [typescript-tslint-plugin] "tslint error Cannot read property 'toString' of null"

A few notes on our environment, in case any of it contains a clue:

We use yarn in the workspace, not npm
We use yarn workspaces, but both typescript and tslint dependencies are declared at the very top level of the workspace, in a file {workspaceRoot}/package.json.
I confirmed that yarn has correctly installed {workspaceRoot}/node_modules/.bin/tsc and {workspaceRoot}/node_modules/.bin/tslint
Also present: {workspaceRoot}/tsconfig.json and {workspaceRoot}/tslint.json
I have followed your secondary instructions to enable workspace library execution from the command palette, though it results in no apparent changes in {workspaceRoot}/.vscode/settings.json. (Not sure if it should?)
Actual .tsx? files live at {workspaceRoot}/frontend/{app}/src/**/*.tsx, with each {app} folder having its own tslint.json that extends the one from the root. So {workspaceRoot}/frontend/{app}/tslint.json contains "extends": ["../../tslint.json"].
We do have an npm global prefix set in all of our users' .npmrc: prefix = /home/{user}/.npm-global, however nobody has a globally installed tslint in there; we've always relied on the workspace tslint binary.

Like I said, I'm not super sure how to provide good debugging information here, but if I had to offer a naive guess, it looks like it's failing to load the workspace binary and trying to fall back to a global one, which we don't have?

Happy to offer any more information I can! Thanks in advance!

mjbvz commented 3 years ago

Could you try putting together a minimal workspace that demonstrates the issue? I suspect this line is the issue but have not been able to reproduce locally: https://github.com/microsoft/typescript-tslint-plugin/blob/2e866e88ce6fad1bb9199dfeb71ead5529e10486/src%2Frunner%2Findex.ts#L482

mjbvz commented 3 years ago

Also, can you please share some info about your environment, such as operating system and VS Code version?

nickjs commented 3 years ago

Local vscode info:

Version: 1.51.1
Commit: e5a624b788d92b8d34d1392e4c4d9789406efe8f
Date: 2020-11-11T01:11:34.018Z (4 wks ago)
Electron: 9.3.3
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Darwin x64 19.6.0 (macOS 10.15.7)

But we're connecting to a Remote SSH workspace where the repo and tsserver/tslint are actually running:

[21:54:17.465] > OpenSSH_8.1p1, LibreSSL 2.7.3
[21:54:17.793] > ready: c0d4b9305461
[21:54:17.858] > Linux 5.4.0-1030-aws #31-Ubuntu SMP Fri Nov 13 11:40:37 UTC 2020
Node.js: 10.16.3
tslint@6.1.3
typescript@4.0.3

I'll set up a repro repo and send that over.

Thanks!

Edit: tsserver spawns with its own version of node:

Info 0    [3:25:41.459] Starting TS Server
Info 1    [3:25:41.459] Version: 4.0.3
Info 2    [3:25:41.459] Arguments: /home/nsmall/.vscode-server/bin/e5a624b788d92b8d34d1392e4c4d9789406efe8f/node /home/nsmall/go/src/github.com/{workspaceRoot}/node_modules/typescript/lib/tsserver.js --useInferredProjectPerProjectRoot --enableTelemetry --cancellationPipeName /home/nsmall/tmp/vscode-typescript1074/6a6cfd4b66853d753595/tscancellation-ea24422e94938d2855f6.tmp* --logVerbosity verbose --logFile /home/nsmall/.vscode-server/data/logs/20201210T024746/exthost1/vscode.typescript-language-features/tsserver-log-SFiSmR/tsserver.log --globalPlugins typescript-vscode-sh-plugin,typescript-tslint-plugin --pluginProbeLocations /home/nsmall/.vscode-server/bin/e5a624b788d92b8d34d1392e4c4d9789406efe8f/extensions/typescript-language-features,/home/nsmall/.vscode-server/extensions/ms-vscode.vscode-typescript-tslint-plugin-1.3.0 --locale en --noGetErrOnBackgroundUpdate --validateDefaultNpmLocation
Info 3    [3:25:41.459] Platform: linux NodeVersion: 12 CaseSensitive: true

nickjs commented 3 years ago

Ok so I set up a super simple reproduction repo: https://github.com/nickjs/vscode-tslint-repro

I think it has something to do with being run on a Remote SSH container. When I open this repo locally on my machine, the plugin works fine. When I open it on a Remote-SSH connection it fails to load with the same error as above. Do you have an easy way to test the extension over SSH? If not I can try to set up an instance.

In the meantime, off I go to figure out how to attach a debugger to tsserver...

nickjs commented 3 years ago

Got it! It's not Remote-SSH, it fails when you have prefix set in ~/.npmrc. I don't know 100% the reason why, but having prefix set to a non-standard path and not having a global tslint binary installed causes both of the global resolveTsLint calls to fail. The first one doesn't matter because it's wrapped in a try/catch, but the second one is not.

I was able to fix this issue for our organization by wrapping https://github.com/microsoft/typescript-tslint-plugin/blob/master/src/runner/index.ts#L256-L258 in a third try/catch. I'll open a PR shortly, but also open to other ideas if there's a better fix.

axiac commented 3 years ago

I'm not sure if this is the same issue or a different issue but I encounter a similar behaviour. Version 1.2.3 of the plugin works fine. Version 1.3.0 produces the following in the log:

Info 36   [18:37:49.559] [typescript-tslint-plugin] "Computing tslint semantic diagnostics for '/Users/vali/.../ConfigurationsLoader.ts'"
Info 37   [18:37:49.560] [typescript-tslint-plugin] "(runTsLint) start"
Info 38   [18:37:49.560] [typescript-tslint-plugin] "(loadLibrary) trying to load /Users/vali/.../ConfigurationsLoader.ts"
Info 39   [18:37:49.560] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Begin - Resolve Global Package Manager Path for: yarn"
Info 40   [18:37:49.780] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Done - Resolve Global Package Manager Path for: yarn"
Info 41   [18:37:49.887] [typescript-tslint-plugin] "tslint error Cannot read property 'toString' of null"

VSCode info:

Version: 1.52.0
Commit: 940b5f4bb5fa47866a54529ed759d95d09ee80be
Date: 2020-12-10T22:46:53.673Z (3 days ago)
Electron: 9.3.5
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Darwin x64 17.7.0

I am using yarn as the Node package manager and it is also the value of the Tslint: Package Manager setting in the plugin settings in VSCode.

The plugin works if npm is configured as Tslint: Package Manager.

I don't have any package installed globally with yarn. Both typescript and tslint are installed in the project. They are also installed globally using npm (but, as I said, I do not use npm in the projects).

mjbvz commented 3 years ago

@nickjs and @axiax Please try the new extension version: 1.3.1. It includes the fix that @nickjs made upstream: https://github.com/microsoft/typescript-tslint-plugin/pull/93

Let me know if you still see issues using the new version

axiac commented 3 years ago

There is no change for me.

If I choose yarn as the value for Tslint: Package Manager setting it fails silently, with the same message in tsserver.log as before:

Info 123  [0:41:58.974] [typescript-tslint-plugin] "(loadLibrary) trying to load /Users/vali/.../ListOfStaticConfigurations.ts"
Info 124  [0:41:58.974] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Begin - Resolve Global Package Manager Path for: yarn"
Info 125  [0:41:58.975] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Done - Resolve Global Package Manager Path for: yarn"
Info 126  [0:41:59.88] [typescript-tslint-plugin] "tslint error Cannot read property 'toString' of null"

If I choose pnpm it also fails silently and logs this:

Info 102  [0:40:32.70] [typescript-tslint-plugin] "(loadLibrary) trying to load /Users/vali/.../ListOfStaticConfigurations.ts"
Info 103  [0:40:32.70] [typescript-tslint-plugin] "(getGlobalPackageManagerPath) Begin - Resolve Global Package Manager Path for: pnpm"
Info 104  [0:40:32.80] [typescript-tslint-plugin] "tslint error Command failed: pnpm root -g\n/bin/sh: pnpm: command not found\n"

which is correct as I never had pnpm installed.

If I choose npm it works fine (version 1.3.0 also worked with this setting).

I use yarn to install the packages in the project. However, if it works with npm as the value for Tslint: Package Manager I'm fine with that. Does this mismatch have any potential to break something?

mjbvz commented 3 years ago

Thanks. Please move this to a new issue since this involves package managers and does not sound like the same issue

nickjs commented 3 years ago

Thanks @mjbvz ! Unfortunately I just pulled 1.3.1 in vscode and am still getting at the same error. Looking at the commit in this repo though, it looks like the typescript-tslint-plugin dependency didn't actually get bumped to 1.0.1? I'm guessing either npm didn't see the patch release as worthy of writing to package-lock.json or something along those lines? Either way, doesn't look like the change is being picked up.

mjbvz commented 3 years ago

Try 1.3.2 which was just published

nickjs commented 3 years ago

Thanks @mjbvz, you're my hero. 1.3.2 works great. Thanks again for all of your wonderful work in the community; may your bugs always be resolved quickly and easily 😃

microsoft / vscode-typescript-tslint-plugin

Latest version fails to start plugin with `Cannot read property 'toString' of null` #138