Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debug: enable js-debug to auto attach #95807

Merged
merged 3 commits into from
Apr 23, 2020

Conversation

connor4312
Copy link
Member

This modifies the debug-auto-launch extension to trigger js-debug as
outlined in #88599 (comment)

Since we now have four states, I moved the previous combinational logic
to a transitions map, which is more clear and reliable. The state
changes are also now a queue (in the form of a promise chain) which
should avoid race conditions.

There's some subtlety around how we cached the "ipcAddress" and know
that environment variables are set. The core desire is being able to
send a command to js-debug to set the environment variables only if they
haven't previously been set--otherwise, reused
the cached ones and the address.

This process (in getIpcAddress) would be vastly simpler if extensions
could read the environment variables that others provide, though there
may be security considerations since secrets are sometimes stashed
(though I could technically implement this today by manually creating
and terminal and running the appropriate echo $FOO command).

This seems to work fairly well in my testing. Fixes #88599.

/cc @isidorn @roblourens

This modifies the debug-auto-launch extension to trigger js-debug as
outlined in #88599 (comment)

Since we now have four states, I moved the previous combinational logic
to a `transitions` map, which is more clear and reliable. The state
changes are also now a queue (in the form of a promise chain) which
should avoid race conditions.

There's some subtlety around how we cached the "ipcAddress" and know
that environment variables are set. The core desire is being able to
send a command to js-debug to set the environment variables only if they
haven't previously been set--otherwise, reused
the cached ones and the address.

This process (in `getIpcAddress`) would be vastly simpler if extensions
could read the environment variables that others provide, though there
may be security considerations since secrets are sometimes stashed
(though I could technically implement this today by manually creating
and terminal and running the appropriate `echo $FOO` command).

This seems to work fairly well in my testing. Fixes #88599.
@connor4312 connor4312 added the debug Debug viewlet, configurations, breakpoints, adapter issues label Apr 21, 2020
@connor4312 connor4312 requested a review from weinand April 21, 2020 23:17
connor4312 added a commit to microsoft/vscode-js-debug that referenced this pull request Apr 21, 2020
connor4312 added a commit to microsoft/vscode-js-debug that referenced this pull request Apr 21, 2020
@weinand weinand added this to the April 2020 milestone Apr 22, 2020
@weinand
Copy link
Contributor

weinand commented Apr 22, 2020

When trying to use this PR I see the issue that js-debug does not automatically start debugging when setting "debug.node.useV3" to true.

Here are the steps:

  • checkout the branch "connor4312/js-debug-auto-attach"
  • run VS Code out of source on a trivial server.js project
  • make sure to use this setting: "debug.node.useV3": false
  • set a breakpoint in the server.js
  • F1 > "auto attach": enable auto attach
  • open the integrated terminal in VS Code (not the JavaScript terminal)
  • from there run "node --inspect server.js"

Observe:
debugger starts and the breakpoint is hit

  • stop debugging
  • changing setting to "debug.node.useV3": true
  • from the same integrated terminal run "node server.js"

Observe:
no debug session starts

I would have expected that js-debug would automatically attach.

The version of the Nightly js-debug is 2020.4.1517.

In addition I see this error:

[59161:0422/164507.675551:INFO:CONSOLE(104)] "%c[Extension Host] %cstack trace: Error: command 'extension.js-debug.setAutoAttachVariables' not found
    at CommandService._tryExecuteCommand (file:///Users/weinand/MS/Projects/vscode/out/vs/workbench/services/commands/common/commandService.js:71:43)
    at file:///Users/weinand/MS/Projects/vscode/out/vs/workbench/services/commands/common/commandService.js:65:51
    at processTicksAndRejections (internal/process/task_queues.js:85:5) %c(at /Users/weinand/MS/Projects/vscode/out/vs/workbench/services/extensions/node/extensionHostProce

@connor4312
Copy link
Member Author

The version of the Nightly js-debug is 2020.4.1517.

The changes into js-debug are still in a PR, I haven't merged/published them yet in case there were any structural changes coming out of this PR. Here's a vsix of that branch, you can install it manually to test this: https://memes.peet.io/img/debug-terminal-js-debug-nightly.vsix

@weinand
Copy link
Contributor

weinand commented Apr 22, 2020

Yep, with that vsix all my scenarios work just fine. Great stuff.

Copy link
Contributor

@weinand weinand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good to me. Just one typo to fix.

@weinand weinand removed their assignment Apr 22, 2020
@connor4312 connor4312 requested a review from weinand April 22, 2020 16:21
connor4312 added a commit to microsoft/vscode-js-debug that referenced this pull request Apr 22, 2020
@connor4312 connor4312 merged commit 66744e3 into master Apr 23, 2020
@connor4312 connor4312 deleted the connor4312/js-debug-auto-attach branch April 23, 2020 16:49
@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt "auto attach" feature to js-debug
2 participants