TypeScript's dependency on electron limits the heap size to a point that breaks monorepos, and tsserver-bridge is no longer a workaround

[MarcCelani-at]

Bug Report

Electron enforces a heap memory limit (due to pointer compression) that causes typescript to crash in large mono-repo. This was originally reported here, and then on the electron side here. One common workaround is to use tsserver-bridge to cause vscode to launch the typescript editor service in node rather than electron. Starting in typescript 4.6, it appears that the tsserver-bridge workaround no longer works.

Error when using tsserver-bridge:

Info 0    [09:17:36.144] Starting TS Server
Info 1    [09:17:36.145] Version: 4.6.4
Info 2    [09:17:36.145] Arguments: /my/path/to/node /my/path/to/.vscode/typescript/lib/_tsserver.js --useInferredProjectPerProjectRoot --enableTelemetry --cancellationPipeName /var/folders/l1/qrb13dbx3l3_wjlh4kgxnpm80000gn/T/vscode-typescript501/8d65dce0fd217a13c1cc/tscancellation-f05eabd1e2d0ae3f8300.tmp* --logVerbosity verbose --logFile /my/path/to/Code/logs/20220509T134031/exthost1/vscode.typescript-language-features/tsserver-log-Y6lofO/tsserver.log --locale en --noGetErrOnBackgroundUpdate --validateDefaultNpmLocation --useNodeIpc
Info 3    [09:17:36.145] Platform: darwin NodeVersion: 16 CaseSensitive: false
Info 4    [09:17:36.145] ServerMode: undefined syntaxOnly: false hasUnknownServerMode: undefined
Info 5    [09:17:36.149] Binding...
Info 6    [09:17:36.153] event:
    {"seq":0,"type":"event","event":"typingsInstallerPid","body":{"pid":28855}}
Err 7     [09:17:36.163] Exception on executing command unknown:

    process.send is not a function

    TypeError: process.send is not a function
        at IpcIOSession.writeMessage (/my/path/to/.vscode/typescript/lib/_tsserver.js:177001:29)
        at IpcIOSession.Session.send (/my/path/to/.vscode/typescript/lib/_tsserver.js:172938:22)
        at IpcIOSession.Session.event (/my/path/to/.vscode/typescript/lib/_tsserver.js:172947:22)
        at IpcIOSession.IOSession.event (/my/path/to/.vscode/typescript/lib/_tsserver.js:176966:48)
        at NodeTypingsInstaller.event (/my/path/to/.vscode/typescript/lib/_tsserver.js:176926:31)
        at Immediate.<anonymous> (/my/path/to/.vscode/typescript/lib/_tsserver.js:176776:71)
        at processImmediate (node:internal/timers:464:21)

Unfortunately, tsserver-bridge is no longer supported (author thinks it is no longer required since you can adjust heap size, but electron limits heap size because it implements pointer compression).

Electron has been so far unwilling to provide builds that disable pointer compression, and even if they did, vscode would need to begin leveraging it.

🔎 Search Terms

tsserver-bridge, heap limit, memory limit, process.send

🕗 Version & Regression Information

• This changed between versions 4.5.4 and 4.6.4

🙁 Actual behavior

Large mono repos hit the memory limit and tsserver-bridge is not a workaround

🙂 Expected behavior

Allow tsserver-bridge to work or get off of electron

[jakebailey]

ipc was recently added to tssever for perf reasons; if you're using VS Code I'm guessing there's a flag that could be passed to disable it? Maybe we (the server or somewhere in the client) need to be checking for send support?

@mjbvz

[mjbvz]

If you pass TSServer --useNodeIpc, it will use node's ipc instead of stdio

tsserver-bridge likely needs to be updated to support ipc too. There's no user facing flag to disable this

[MarcCelani-at]

tsserver-bridge likely needs to be updated to support ipc too.

what does this mean? The whole tsserver-bridge package is pretty, short, would appreciate a pointer of what to change here.

The stack trace I shared is already using IpcIOSession (not IOSession), so I think that means it is already using node ipc? I tried passing the flag by patching tsserver-bridge, and I the flag is being passed (according to arguments that are logged in tsserver), but that didn't fix it. Something else needs to change too?

[jakebailey]

The bridge code creates a script in place of tsserver that spawns the real tsserver as a subprocess, inheriting stdin/stdout/stderr so that subprocess can communicate with the client. But newer versions of TS and VS Code support IPC, and so the wrapper script in the middle has to proxy the IPC traffic as well if --useNodeIpc was passed (and probably run it via the version of spawn that enables IPC, too).

Otherwise, the client and server will attempt to communicate over IPC and then crash in the IPC code, as your trace shows.

[andrewbranch]

We discussed this today and decided VS Code should possibly let users opt into spawning TS Server in their own installed version of Node (and inform TS Server so it can communicate over STDIO as before, IIUC). @mjbvz was going to see if other language extensions have run into something similar. Matt, can I transfer this issue to VS Code for now?

[MarcCelani-at]

Is there a related issue on vscode side?

[RyanCavanaugh]

Assigning to @mjbvz since I don't have rights to transfer this to the VS Code repo 😅

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[MarcCelani-at]

Can we keep this open until the vscode team actively takes ownership?

[MarcCelani-at]

Thanks for taking ownership of this task in the vscode project.

I'm trying to understand if our team needs to work around this issue by patching tsserver-bridge or not. Is this issue a priority for vscode, and is it easy to fix?

[mjbvz]

Our proposal is to let users specify a path to node and run TSServer using that. It's not a priority though so would definitely appreciate a PR :)

[MarcCelani-at]

@mjbvz : Can you point me to relevant code? I've never touched this codebase.

[MarcCelani-at]

Okay, here is a rough proposal...

New config:
typescript.runtime: string | null // Path to runtime bin or the name of a binary that is set in PATH

I'll update and rename serverProcess.electron.ts to consume these configurations and change how the fork happens to use this runtime value.

[mjbvz]

Yes that sounds like the correct approach. A few points:

• For security reasons, we can't just use this value from the workspace settings (because it could point to an evil version of node). You can take a look at how the php.validate.executablePath setting for a similar example of how this should work

• I'd even say we only want the new setting to be configurable as user setting and never as a workspace setting. We don't expect many users to configure it and if they do, they likely want the custom version of node to always be used

• We need to log when a custom node version is being used (we already log the TS version). This will help us track down if a custom node version is causing issues

[MarcCelani-at]

Is the "validate" word a naming convention, and what does it imply? Where does the validation logic happen?

[mjbvz]

validate is for error reporting. It's not relevant here (I was just sharing an example of another setting that contains a path to an executable)

[mjbvz]

Actually let's merge this issue into #127105 which was already tracking this

Duplicate of #127105

[mjbvz]

Reopening to track letting users point to a node executable for running tsserver

[gabritto]

@mjbvz regarding your comment above:

We need to log when a custom node version is being used (we already log the TS version). This will help us track down if a custom node version is causing issues

Where should that be logged?

[jakebailey]

Reopening to track letting users point to a node executable for running tsserver

I hadn't noticed, but #172719 is actually also the same request!

[VSCodeTriageBot]

Friendly ping! Looks like this issue requires some further steps to be verified. Please provide us with the steps necessary to verify this issue.

[mjbvz]

For verification, this is a duplicate of #172719. Marking as verified based on that issue