microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.06k stars 29.23k forks source link

using "runtimeVersion" overloads console on launch #198550

Closed guycnicholas closed 3 months ago

guycnicholas commented 11 months ago

Type: Bug

We run NodeJS servers and in our launch.json file we generally like to put the runtimeVersion. What happens though is that it will dump the PATH properties to the console, which exceeds the character limit of a command, and then the debugger never starts. The workaround is to remove "runtimeVersion" from the launch properties. Here is a sample from the terminal window when I tried to launch, and what you will notice is that the text ends with "and" which was supposed to be "android" /usr/bin/env ENVIRONMENT_NAME=local 'PATH=/Users/nicholas/.nvm/versions/node/v12.14.1/bin:/Users/nicholas/.nvm/versions/node/v12.14.1/bin:/Users/nicholas/.pyenv/shims:/Users/nicholas/Library/pnpm:/Users/nicholas/.yarn/bin:/Users/nicholas/.config/yarn/global/node_modules/.bin:/Library/Frameworks/Python.framework/Versions/3.7/bin:/Users/nicholas/.nvm/versions/node/v18.16.0/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:~/.dotnet/tools:/usr/local/git/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/Applications/Xamarin Workbooks.app/Contents/SharedSupport/path-bin:/opt/local/bin:/opt/local/sbin:/Users/nicholas/Development/SDKs/android-sdk-macosx/platform-tools/:/Users/nicholas/Development/SDKs/and~/Development/CCHome/cchome-config-server: /usr/bin/env ENVIRONMENT_NAME=local 'PATH=/Users/nicholas/.nvm/versions/node/v12.14.1/bin:/Users/nicholas/.nvm/versions/node/v12.14.1/bin:/Users/nicholas/.pyenv/shims:/Users/nicholas/Library/pnpm:/Users/nicholas/.yarn/bin:/Users/nicholas/.config/yarn/global/node_modules/.bin:/Library/Frameworks/Python.framework/Versions/3.7/bin:/Users/nicholas/.nvm/versions/node/v18.16.0/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:~/.dotnet/tools:/usr/local/git/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/Applications/Xamarin Workbooks.app/Contents/SharedSupport/path-bin:/opt/local/bin:/opt/local/sbin:/Users/nicholas/Development/SDKs/android-sdk-macosx/platform-tools/:/Users/nicholas/Development/SDKs/and

VS Code version: Code 1.84.2 (Universal) (1a5daa3a0231a0fbba4f14db7ec463cf99d7768e, 2023-11-09T10:52:33.687Z) OS version: Darwin arm64 23.1.0 Modes:

System Info |Item|Value| |---|---| |CPUs|Apple M1 Max (10 x 24)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled| |Load (avg)|5, 5, 4| |Memory (System)|32.00GB (0.13GB free)| |Process Argv|--crash-reporter-id 8a8708a2-857f-4160-b1d7-75d09a14ed9d| |Screen Reader|no| |VM|0%|
Extensions (59) Extension|Author (truncated)|Version ---|---|--- logcat|abh|0.0.7 android-dev-ext|ade|1.3.2 cloudformation-yaml-validator|cha|0.3.15 npm-intellisense|chr|1.4.5 aws-cloudformation-yaml|Dan|0.2.2 vscode-eslint|dba|2.4.2 fileheadercomment|doi|0.0.5 vscode-jest-runner|fir|0.4.69 flow-for-vscode|flo|2.2.1 gc-excelviewer|Gra|4.2.58 todo-tree|Gru|0.0.226 duk-debug|Har|0.5.6 vscode-test-explorer|hbe|2.21.1 rest-client|hum|0.25.1 vscode-hexdump|ili|1.8.2 vscode-peacock|joh|4.2.2 mdhelper|jos|0.0.11 vscode-position|jtr|1.1.2 vscode-jest-test-adapter|kav|0.8.1 vscode-cfn-lint|kdd|0.25.1 vscode-docker|ms-|1.28.0 csharp|ms-|2.10.28 vscode-dotnet-runtime|ms-|2.0.0 vscode-edge-devtools|ms-|2.1.3 isort|ms-|2023.10.1 python|ms-|2023.20.0 vscode-pylance|ms-|2023.11.10 jupyter|ms-|2023.10.1100000000 jupyter-keymap|ms-|1.1.2 jupyter-renderers|ms-|1.0.17 vscode-jupyter-cell-tags|ms-|0.1.8 vscode-jupyter-slideshow|ms-|0.1.5 remote-containers|ms-|0.321.0 azure-account|ms-|0.11.6 cpptools|ms-|1.18.5 js-debug-nightly|ms-|2023.11.1317 makefile-tools|ms-|0.7.0 test-adapter-converter|ms-|0.1.8 vscode-js-profile-flame|ms-|1.0.8 vscode-typescript-next|ms-|5.4.20231116 vsliveshare|ms-|1.0.5892 vsliveshare-pack|ms-|0.4.0 debugger-for-edge|msj|1.0.15 vscode-jest|Ort|5.2.3 vscode-code-outline|pat|0.2.1 java|red|1.24.0 json-to-js|ren|0.2.0 bash-debug|rog|0.3.9 code-spell-checker|str|3.0.1 vscode-stylelint|sty|1.3.0 intellicode-api-usage-examples|Vis|0.2.8 vscodeintellicode|Vis|1.2.30 vscode-java-debug|vsc|0.55.0 vscode-java-dependency|vsc|0.23.2 vscode-java-pack|vsc|0.25.15 vscode-java-test|vsc|0.40.1 vscode-maven|vsc|0.43.0 vscode-icons|vsc|12.6.0 vscode-import-cost|wix|3.3.0
A/B Experiments ``` vsliv368cf:30146710 vsreu685:30147344 python383cf:30185419 vspor879:30202332 vspor708:30202333 vspor363:30204092 vslsvsres303:30308271 vserr242:30382549 pythontb:30283811 vsjup518:30340749 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 vscod805:30301674 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 vsaa593cf:30376535 pythonvs932:30410667 py29gd2263cf:30880073 vscaac:30438847 vsclangdc:30486549 c4g48928:30535728 dsvsc012:30540252 pynewext54:30695312 azure-dev_surveyone:30548225 2e4cg342:30602488 f6dab269:30613381 showlangstatbar:30737416 pythonfmttext:30731395 fixshowwlkth:30771522 showindicator:30805244 pythongtdpath:30769146 i26e3531:30792625 welcomedialog:30887143 pythonnosmt12:30797651 pythonidxpt:30866567 pythonnoceb:30805159 synctok:30869157 dsvsc013:30795093 dsvsc014:30804076 dsvsc015:30845448 pythontestfixtcf:30871695 pythonregdiag2:30871582 pyreplss2:30886141 pythonmypyd1:30879173 pythoncet0:30885854 2e7ec940:30885897 pythontbext0:30879054 accentitlementsc:30887149 dsvsc016:30886110 dsvsc017:30886112 dsvsc018:30886114 aa_t_chat:30882232 ```
connor4312 commented 11 months ago

This is VS Code itself; for runtimeVersion, we intentionally want to prepend to the PATH, but it seems like that can result in a command line length above the maximum. We could work around it in js-debug by referencing the executable directly, but the underlying issue is still present for other extensions.

@Tyriar do you have any recommendations here? Currently we create and send a command with the environment an extension asks us to use to launch the program.

related: https://github.com/microsoft/vscode/issues/100538, https://github.com/microsoft/vscode/issues/60971, probably others, maybe solvable as we're a lot smarter with the terminal nowadays...

justschen commented 7 months ago

@guycnicholas could you verify that this is fixed in the latest Insiders build?

or also @connor4312 if we could get verification steps?

connor4312 commented 7 months ago
  1. Have a node.js launch config with something in the env key, and console: "integratedTerminal"
  2. Run it
  3. Verify you no longer see a massive dump of the environment when the terminal is created
gnichola commented 7 months ago

@justschen Yepper, it looks glorious! Thanks

connor4312 commented 7 months ago

Thanks for confirming!

guycnicholas commented 7 months ago

@connor4312 Argggg....there is a problem, and I am not sure how our team can work around it. I was going to log a bug, but I think it is this fix causing it. Scenarios: With the newest 1.88 release the debugging/console behavior changed a bit and broke.

  1. launch with "console": "integratedTerminal" - result breakpoints do not get hit, output goes to terminal window.
  2. launch with "console": "internalConsole" - BPs get hit, output goes to Debug console
  3. launch with no console item - BPs get hit, output goes to Debug console

The 1.87.2 release hits BPs on all 3 scenarios, and the output behaves the same as 1.88

The issue is, we typically run from the integrated terminal as we paste our AWS Klam credentials there so the app can pick them up. The other terminal options do not allow for that as I am not sure how to get to them.

I didn't even consider something like this, so my testing was limited to launching some of my servers. I then didn't continue using the early build as it didn't pickup the settings from my released version, so I swtiched back. Are there any insights you can provide?

connor4312 commented 7 months ago

It sounds like there is some behavioral difference in your application regarding whether it's run with a PTY or not. I would need more details to know how to recommend a fix.

A common way to deal with credentials is using an envFile containing environment variables you wish to be set when your program is run.

guycnicholas commented 7 months ago

VSCodeTest.zip I have recommended to my teammates that they keep the 1.87 version around until I fix happens, which I assume would not happen until the April release at a minimum. The bug being the lack of hitting any BPs when using the integrated term. As to "workaround", I was more hoping there could be a way to keep the debugger attached. Presently, it looks like it connects and disconnects immediately. Here is the output from my sample.

/Users/nicholas/.nvm/versions/node/v16.16.0/bin/node ./test.js 
Debugger attached.
Waiting for the debugger to disconnect...
~/Development/VSCodeTest:  /Users/nicholas/.nvm/versions/node/v16.16.0/bin/node ./test.js 
9
27

Do I need to log another bug on this?

connor4312 commented 7 months ago

This does work for me, it's possible the environment variables are not getting applied in your terminal correctly:

guycnicholas commented 7 months ago

I think we are having a Miss-Q on this. The problem isn't that the integrated terminal doesn't have env vars. The problem is the debugger doesn't attach. Try to put a breakpoint in the code and see if you get there.

connor4312 commented 7 months ago

The debugger uses environment variables to set up its attachment to the program. Breakpoints work for me, using zshell on macos. If there's some setup in your environment which prevents those from getting through (either a shell that might not accept environment variables, or a configuration that overwrites NODE_OPTIONS in your dotfiles) attachment won't work. You can see this if you log process.env.NODE_OPTIONS in a correctly-attached debug sessions.

guycnicholas commented 7 months ago

Oh...sorry. I get: --require /Users/nicholas/.vscode/extensions/ms-vscode.js-debug-nightly-2024.3.2217/src/bootloader.js --inspect-publish-uid=http I have bash as the default..let me switch.

connor4312 commented 7 months ago

hm, okay, that should work find. Can you grab a log?

If you're able to, add "trace": true to your launch.json and reproduce the issue. The location of the log file on your disk will be written to the Debug Console. Share that with us.

⚠️ This log file will not contain source code, but will contain file paths. You can drop it into https://microsoft.github.io/vscode-pwa-analyzer/index.html to see what it contains. If you'd rather not share the log publicly, you can email it to connor@xbox.com

guycnicholas commented 7 months ago

I don't get much in the file, the attached image shows what file I am looking at, and it is the newest, but it's contents are only: {"cwd":"/Users/nicholas/Development/VSCodeTest","processId":45115,"nodeVersion":"v16.16.0","architecture":"x64"} image (2)

connor4312 commented 7 months ago

Ohhh, I see what's happening. This is unfortunate.

You have nvm in your dotfiles, and it runs an npm command to do 'something' at some point, which is when the debugger attaches. So the debugger attaches, it runs, and the debugger thinks the session is over and cleans up its mess (including the unix socket used to communicate with the debugee.) Then the real program runs, sees the socket is gone, and thinks that it must no longer be in debug mode.

I'm going to back this change out in a patch release and figure out how to reimplement it. Sorry for the hassle.

guycnicholas commented 7 months ago

Thanks for your effort and glad you figured it out. Setting the runtimeVersion is there because it is desirable to run each server using the version contained in its Docker container, which then requires us to have multiple versions on our machines...which of course, leads to NVM.

connor4312 commented 7 months ago

Very much an aside, but a while back I switched to fnm which has a much lower shell startup time impact and works on Windows. We support it in runtimeVersion too. I can recommend it :)

connor4312 commented 6 months ago

VS Code 1.88.1 has been released with the fix

danmana commented 3 months ago

I can still reproduce this issue on Macos. image

VSCode info

Version: 1.91.0
Commit: ea1445cc7016315d0f5728f8e8b12a45dc0a7286
Date: 2024-07-01T18:53:23.353Z
Electron: 29.4.0
ElectronBuildId: 9728852
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Darwin arm64 23.5.0

Not sure if relevant, my terminal is zsh.

launch.json

{
          "type": "node",
          "request": "launch",
          "name": "Test integratedTerminal",
          "skipFiles": [
              "<node_internals>/**"
          ],
          "program": "${workspaceFolder}/index.js",
          "console": "integratedTerminal",
          "runtimeVersion": "18"
      }

Checking the Terminal logs I can see that the generated command is 1437 characters long (due to long PATH in env) However only the first 1024 characters are sent correctly and parsed

image

Removing the runtimeVersion works (using the same Node v18.20.3) The command generated is only 671 characters long (strangely it does not contain PATH?)

Even more strangely, if I keep this terminal open in vscode, add back the runtimeVersion and run the program again, now it works with 1437 character command! Closing the terminal, and running again fails.

So the first command in a new terminal is limited to 1024 characters ??

image

guycnicholas commented 3 months ago

Likewise. I am on 1.91.1 and I tried all 3 consoles, integrated and external both write a terminated path to the terminal and fail to continue to launch. When set to internal, it just launches and quits. I am still using NVM.

danmana commented 3 months ago

Same, using nvm. So far my solution is to:

  1. remove "runtimeVersion" from launch.json
  2. make sure I run nvm use to load the correct node version for the project.

Or use "console": "externalTerminal"

guycnicholas commented 3 months ago

same, but the point of having the version in there is that we then match it to the version in the docker container we build with, and then all devs are ensured of running with the correct version without having to think about it.

danmana commented 3 months ago

I cloned the VS Code repo and ran it from the source code to try and debug the issue, but I couldn't reproduce the problem in debug mode.

I'm not sure if the issue is fixed in the latest version on the main branch or if there is a difference between running a production build of VS Code versus a development build. The development build seems slower, so if the issue is related to timing or a race condition, it might not occur in the slower version.

I'll try running from the 1.91.0 tag later, to see if that works or not

connor4312 commented 3 months ago

This issue is not fixed, it's still open

danmana commented 3 months ago

I checked out 1.91.0 and ran it from source code. With this version I can reproduce the issue 8/10 times.

If I add a breakpoint and pause for a second before sendText, it works. So I can't inspect it to see what fails.

If I add a 1s timeout before sendText here, it seems to work every time https://github.com/microsoft/vscode/blob/1.91.0/src/vs/workbench/api/browser/mainThreadTerminalService.ts#L208

Might also be related with https://github.com/microsoft/vscode/issues/38137

connor4312 commented 3 months ago

I don't think there's any solution to this, if someone is smarter and comes up with an idea please let me know.