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

Test: js-debug auto attach #105771

Closed
3 tasks done
connor4312 opened this issue Aug 31, 2020 · 9 comments
Closed
3 tasks done

Test: js-debug auto attach #105771

connor4312 opened this issue Aug 31, 2020 · 9 comments

Comments

@connor4312
Copy link
Member

connor4312 commented Aug 31, 2020

Refs: microsoft/vscode-js-debug#703

Complexity: 5

Create Issue


This month we brought js-debug auto attach back. This includes some behavior changes:

  • You can now configure whether js-debug auto attach will be used via the debug.javascript.usePreviewAutoAttach setting. (defaults to true)
  • By default, auto attach will connect to 'user' node scripts and common tools. It does this by not attaching to any script running in a node_modules, excluding some common tools like mocha, ts-node, etc.
    • The "common tools" and scripts is configurable in debug.javascript.autoAttachSmartPattern
    • The attachment mode is configurable debug.javascript.autoAttachFilter
    • Verify this functionality works and makes sense to you.
  • Auto attach happens through environment variables which are persisted per-workspace.
    • Toggle attach between different settings and move around workspaces. Verify that the correct state is reflected in each workspace.
    • You can verify that auto attach is fully 'disabled' by checking that echo $NODE_OPTIONS / echo $env:NODE_OPTIONS is not present
  • There are changes around when we decide we can enable auto attach. Primarily, we cannot verify the user is running Node 12 and there's a space in the workspace storage folder, auto attach will not be enabled and we'll show a nag warning.

Additional ask: please leave auto attach "On" or "Off" (not disabled) for the remainder of endgame as you go about your business, and let me know if you run into any issues using the terminal or running node scripts.

@isidorn
Copy link
Contributor

isidorn commented Sep 1, 2020

@connor4312

  • Regarding "Auto attach happens through environment variables which are persisted per-workspace". For me autoAttach is simply written into my user settings so it is preserved across Workspaces. Can you please clarify what I am missing here?
  • Regarding "There are changes around when we decide we can enable auto attach". For me Auto Attach is never enabled by default when I open a folder via new user data dir. I always have to explicitly enable it. Can you please clarify what you meant by this part?

@joaomoreno
Copy link
Member

joaomoreno commented Sep 1, 2020

@connor4312 This TPI wasn't obvious to me. I never used this area of Code so I didn't understand what it was about. I also didn't know how to start it. This was even after I read the original feature request item. I had @isidorn talk me through the whole thing in Teams.

I believe some introduction is always needed, eg:

This month we brought js-debug auto attach back. Auto attach is the feature that lets VS Code automatically attach to running Node.JS scripts when they run from the integrated terminal. You can use it from two separate entry points:

  1. Enable debug.node.autoAttach and run node SCRIPT from any integrated terminal.
  2. Launch a new JavaScript Debug Terminal and run node SCRIPT from it.

This includes some behavior changes:
[...]

@isidorn
Copy link
Contributor

isidorn commented Sep 1, 2020

My 2 cents: the node auto attach feature should be unified with the JavaScript Debug Terminal. To make a smoother unified experience. I understand this will probably not make all the users happy but just feels like they should be one thing to me.

@joaomoreno
Copy link
Member

joaomoreno commented Sep 1, 2020

Yeah, for something that should be a smooth experience (auto attaching to a node process), I must say the following points make it confusing:

  • There are two non-obvious entry points (enable a setting and use terminal or use javascript terminal). Can't we just have one obvious entry point? Eg: a notification which opens up the first time a node process is spawned from any terminal Do you wanna debug? Yes, Always, No, Don't Bother Me Again.
  • Settings intersect and have complex dynamics between them. Setting values and descriptions are confusing. But then again, maybe users won't be spending much time playing around with these settings.

@joaomoreno
Copy link
Member

there's a space in the workspace folder, auto attach will not be enabled and we'll show a nag warning

I was able to auto attach to a node --inspect script.js process running off a workspace folder with a space in it. 🤔

@joaomoreno joaomoreno removed their assignment Sep 1, 2020
@connor4312
Copy link
Member Author

connor4312 commented Sep 1, 2020

Thank you for the feedback and bugs!

Regarding "Auto attach happens through environment variables which are persisted per-workspace". For me autoAttach is simply written into my user settings so it is preserved across Workspaces. Can you please clarify what I am missing here?

The environment variables are set per-workspace, regardless of where the setting is stored. So if you have auto attach in your settings and you open a new workspace, the built-in 'debug auto launch' extension will activate js-debug and ask it to set up the environment variables for that workspace.

This isn't a concept really exposed to users, but I mentioned it because it's useful to understand how variables are set / when the ⚠️ comes up as you're going through the TPI.

Regarding "There are changes around when we decide we can enable auto attach". For me Auto Attach is never enabled by default when I open a folder via new user data dir. I always have to explicitly enable it. Can you please clarify what you meant by this part?

Are you setting auto attach to "on" in your user settings? Note that the Toggle Auto Attach command will persist in the workspace setting by default.

I believe some introduction is always needed

Will do for next time, apologies for the confusion 👍

My 2 cents: the node auto attach feature should be unified with the JavaScript Debug Terminal. To make a smoother unified experience

Let's discuss this in #105852

Settings intersect and have complex dynamics between them

This is a temporary state -- I elaborated a bit more on it in microsoft/vscode-js-debug#732 (comment). Let's continue discussion in that issue.

I was able to auto attach to a node --inspect script.js process running off a workspace folder with a space in it. 🤔

Sorry, this should have said workspace storage path.

@isidorn
Copy link
Contributor

isidorn commented Sep 1, 2020

Are you setting auto attach to "on" in your user settings? Note that the Toggle Auto Attach command will persist in the workspace setting by default.

Oh my. I find this a bit confusing. So if I manualy first set it in user settings you will always write it there, otherwise you would do this in the workspace settings - making the whole repository dirty. I am not a fan of automatic writing in workspace settings...

@connor4312
Copy link
Member Author

I think someone brought that up before. That is existing behavior for that command, but as part of the polish next month I can adjust that.

@isidorn
Copy link
Contributor

isidorn commented Sep 1, 2020

Great, thanks. Let me know if you would prefer that I create an issue for that.

@eamodio eamodio removed their assignment Sep 2, 2020
@eamodio eamodio closed this as completed Sep 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants