Ensure node-based debug adapters spawn same node exec as Theia

[veredcon]

Signed-off-by: Vered Constantin vered.constantin@sap.com

Sometimes the container/machine running Theia has more than one node executable available.

The node executable used to run Theia should also be used to run node-based debug adapters; whereas, another node executable can be used to run/debug user applications.

Debug adapters specify their runtime in their corresponding package.json file (e.g. link):

"contributes": {
  "debuggers": [
    {
       "runtime": "node"
    }
}

The debug adapter typically does not specify the full path of the node executable, which means that Theia uses the first node executable on the PATH environment variable to spawn the debug adapter. However, this might not be the same node executable that was used to run Theia.

For us at SAP, this is critical, as we expose node executables to the Theia container, and those executables might run remotely on other containers that have no access to the debug adapter code (which resides on the Theia container).

This pull request ensures that if a debug adapter doesn't specify a fully qualified node runtime, theia would spawn the debug adapter using the same node executable that was used to launch Theia.

This change should not adversely affect existing flows, but rather add support for the requirement detailed above.

How to Test:

1. Use nvm to install two node versions. For example 10.16.0 and 12.2.0
2. Set 12.2.0 as active version: nvm use 12.2.0
3. Launch theia using different node version from the active node version on the PATH:
   can do this by adding:
   "runtimeExecutable": "c:/Users/<>/AppData/Roaming/nvm/v10.16.0/node.exe"
   to the launch configuration of "Launch Browser Backend".
4. Test debug node application works. The debug node adapter process should use node v10.16.10
   same as theia process and not the node version as configured in PATH.
   I debugged the flow to verify this and also used Windows PowerShell with the following
   command:
   get-process | get-item -erroraction silentlycontinue | format-table name, directory | grep node
   I could see processes from directory "C:\Users<>\AppData\Roaming\nvm\v10.16.0" but without
   this PR I see that after clicking "Launch" on my node application and the application is running,
   there are node processes from the PATH "C:\Program Files\nodejs"

This issue is similar to #5492 and #5508.

[vince-fugnitto]

@veredcon thank you for your first contribution!
There are a couple of things that need to be completed before we can accept your changes:

• please be sure to squash your commits (unless they can/need to be separated logically)
• please be sure to sign-off your commit(s)
• please be sure to sign the Eclipse Contributor Agreement (ECA)

Also, it looks like the following code as-is has errors:
https://travis-ci.com/eclipse-theia/theia/jobs/295775177#L705-L707

[akosyakov]

@tolusha Could you give a look please? You know this area better, thank you!

[akosyakov]

@veredcon Please update the description to reflect our template: https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#pr-template

How to test section is important. It shows us that you took time to tests for regressions and document a way for others how to do it for next time when we have to change this area.

[kittaakos]

Please keep in mind, if we go with the proposed change, we will use Node.js bundled with electron. Which is not what you want either.

[veredcon]

Thanks @kittaakos for your review!
Why is it an issue? Do you have a suggestion?
Thanks!

[kittaakos]

Why is it an issue?

Not an issue but not necessarily what you want. For example, If you use mvn nvm and you have Node.js 10.15.3 on the PATH and you start an electron application you won't ever use your Node.js 10.15.3 from the PATH as you would expect, but you will use the Node.js v10.11.0 which is shipped with electron@4.2.12.

edit: mvn -> nvm

[akosyakov]

Please keep in mind, if we go with the proposed change, we will use Node.js bundled with electron. Which is not what you want either.

@kittaakos But only by default? A user still can change launch configuration to configure another npm version: https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_multi-version-support

I wonder which version VS Code is using by default and how it gets it.

[kittaakos]

I wonder which version VS Code is using by default and how it gets it.

Good point, the tiny research should be part of the PR.

[akosyakov]

Actually ignore what I've referenced. Referenced version is to be used by the debug adapter process to run a user program. A PR is about the node for the debug adapter process itself. Any node which can run node debug adapter should work. I would say we have to test it against Electron and if it works then it should be fine.

[veredcon]

Hi @akosyakov - I've tested it with Electron and was able to debug node application successfully.

[akosyakov]

ok, will test tomorrow, if nobody does it before

[akosyakov]

It looks good, thank you for clean up!

I've verified that node based debug adapters are still working for electron and browser targets.

[veredcon]

Thank you very much for your review and responsiveness!