[wip] implement runtimeExecutable version detection

[refack]

A very rudimentary version detection mechanism.
Let's discuss?

Fix nodejs/node#12364
Fix microsoft/vscode#24611
Ref: https://youtrack.jetbrains.com/issue/WEB-26568#comment=27-2120728

[refack]

1. I believe it's better to detect only "onchange" of args.program and cache the result.
2. Could add a if major < 6: throw "we are incompatible"

[roblourens]

I can respond more later, but what if runtimeExecutable is electron? or npm?

[refack]

First of all can we agree that 90% of cases it's node.
So we're talking about edge cases, usually created by users who know what they are doing, we could give them a parameter to set.

For more common cases:

1. npm has --versions

C:\code\3party\node-gyp> npm --versions
{ 'node-gyp': '3.6.0',
  npm: '4.5.0',
  ares: '1.10.1-DEV',
  cldr: '30.0.3',
  http_parser: '2.7.0',
  icu: '58.2',
  modules: '51',
  node: '7.9.0',
  openssl: '1.0.2k',
  tz: '2016j',
  unicode: '9.0',
  uv: '1.11.0',
  v8: '5.5.372.43',
  zlib: '1.2.11' }

2. Electron is a PITA, but we could find the version of its node.dll

I'm happy to write all this code, just need some pointers from you:

1. How would you detect runtimeExecutable change
2. where would you cache the value
3. How would you add an override switch

P.S. From the feedback I got from JetBrains the issue of unabling to debug non-node.exe was not raised by their users (they've been doing version detection for a while).

[roblourens]

Yeah, most of the time we could check the version of 'node' that's on the path, and it will work out. But this introduces another tricky pitfall that will catch people.

• We can't detect launch config changes or cache the result, so any detection work happens on every run
• I don't want to keep special cases for everything that runtimeExecutable can be, because guessing like that will always fall apart. One day I add a special case for npm, and the next day, people use something called yarn :)
• And also, I don't want to invoke a program in an unexpected way when I don't know what it actually is.
• Another option is just trying --inspect-brk and looking for the "bad option" stderr message, but I don't know that I'll always get that. Electron at least won't print it. (Electron doesn't support --inspect yet, but presumably will soon.)
• Make the user configure it - I've found that having two debug protocol is confusing enough for users. Having 3 flavors in play is too much.

If we don't detect whether --inspect-brk is supported, we would drop support for any Node version that doesn't have --inspect-brk (backported to 6.x) - I think it's too quick to make a change like that. We would have to tell people to upgrade to 6.11, or whatever, but then if it doesn't work in 7.0 I think that would be confusing.

[refack]

My comment in nodejs/node#12364 was that you're talking edge cases.

• A user that updates to a semver-major version of their platform but dosn't update the IDE.
• Special runtimeExecutable

I don't want to keep special cases for everything that runtimeExecutable can be, because guessing like that will always fall apart. One day I add a special case for npm, and the next day, people use something called yarn :)

Obviously but software is always an 90/10 game, if you can cover 10% of the cases you help 90% of your users.

And also, I don't want to invoke a program in an unexpected way when I don't know what it actually is.

Understood. We can do the check only if runtimeExecutable.amtch('node(.*).exe')

Make the user configure it - I've found that having two debug protocol is confusing enough for users. Having 3 flavors in play is too much.

As a fall back to an edge case of an edge case (user using special runtimeExecutable that wasn't updated)

[refack]

@roblourens even though it seems like --debug-brk is going to stay, I still recommend we work on version detection.

1. instead of running runtimeExecutable we could read the PE header
2. we could cache the output based on the value of runtimeExecutable
   I think it's safe to assume that the file behind runtimeExecutable will not change during a VSCode session.
3. We could make give informative errors (like about electron not supported)

[roblourens]

I feel like this is adding a lot of complexity to something that's difficult for users to understand, and difficult to debug via github issue.

I think it's safe to assume that the file behind runtimeExecutable will not change during a VSCode session.

I don't think that's safe.

[refack]

🤷It's your call.
My offer to help still stands, if you'll need it in the future.

P.S. You could change the args to --inspect --debug-port=XXX --debug-brk if you want to be more explicit. This combination will be not be removed

[roblourens]

Thanks for the offer :)