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

NodeJs Debug, add some default settings #69959

Closed
katlimruiz opened this issue Mar 7, 2019 · 10 comments · Fixed by microsoft/vscode-node-debug#196
Closed

NodeJs Debug, add some default settings #69959

katlimruiz opened this issue Mar 7, 2019 · 10 comments · Fixed by microsoft/vscode-node-debug#196
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@katlimruiz
Copy link

katlimruiz commented Mar 7, 2019

When creating a launch.json with node, the following settings should be added by default: outputCapture and skipFiles.

        {
            "type": "node",
            "request": "launch",
            "name": "Launch Program",
            "program": "${workspaceFolder}\\index.js",
            "outputCapture": "std",
            "skipFiles": [
                "<node_internals>/**"
            ],
        }

This way when stepping in and out you dont find trapped in the async hooks, and so on.

If not possible both, at least skipFiles.

@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality labels Mar 7, 2019
@roblourens
Copy link
Member

outputCapture just exists for some special scenarios, I wouldn't recommend it as a default.

It might be good to show off skipFiles by default. I think not enabled by default, but included in new generated configurations by default. What do you think @auchenberg or @weinand?

@auchenberg
Copy link
Contributor

@roblourens I don't have strong opinions about skipFiles and outputCapture, but instead of providing more default options that get's written to all launch.json's, wouldn't we update the internal defaults? I'd like to keep our debug configuration as minimal as possible.

@roblourens
Copy link
Member

I can't decide whether adding that as an internal default is too "opinionated" or not. Debugging inside node internal modules is a legitimate thing to do, but ending up in them by accident, especially the async ones, is annoying. So adding it to the autogenerated launch configs could be a way to advertise it without making people have to google for how to turn it off. But yeah it is counter to the goal of making the launch config minimal. So anyway I probably won't do anything about this right now.

@DABH
Copy link

DABH commented Mar 24, 2019

Hi @roblourens -- just wondering about outputCapture or console defaults in particular, based on e.g.
#19750 (comment)
#34702 (comment)
In a perfect world I'd like winston logging library users to have success in vsc out-of-the-box, and it's awesome vsc does have the necessary support.

Is there any downside to default "console": "internalConsole" or "outputCapture": "std"? Changing the console sounds dramatic (maybe it's not though?), but making sure appropriate output is captured sounds like a strict positive? (I'm naive here, haven't looked into exactly what that option is doing internally.)

@roblourens
Copy link
Member

"console": "internalConsole" is the default. What issue are you having?

"outputCapture": "std" has the downside that you only get logs as text, not the live interactive objects that we get through the debug connection.

@DABH
Copy link

DABH commented Mar 25, 2019

So I'm trying to Debug the following script:

const { createLogger, format, transports } = require('winston');
const { combine, timestamp, prettyPrint } = format;

const logger = createLogger({
  format: combine(
    timestamp(),
    prettyPrint()
  ),
  transports: [new transports.Console()]
})

logger.info('hey');
console.log('hi');

I'd expect to see both messages on the console, but since winston's console transport writes directly to stdout, only hi is printed in the vsc debug terminal. Adding "outputCapture": "std" to my launch.json debug config makes both messages print in the Debug terminal. But I sympathize with users who are confused when they try to debug their code but don't see log messages being printed -- not obvious this setting is needed without googling.

I think I understand your point about the downside of "outputCapture": "std". Seems a bit odd only one or the other behavior is possible -- wouldn't it be possible to capture the union of the std output and interactive objects, if that makes sense?

@roblourens
Copy link
Member

Then in a normal case, logs will show up twice. It's not possible to dedupe them because they can be formatted differently.

@ccinelli
Copy link

ccinelli commented Apr 5, 2019

  1. It took me a while to figure out what was going on here: with the npm module debug, that is (or at least used to be) pretty popular, a debug("something") does not show anything anywhere. Adding "outputCapture": "std", shows the output but without color and the module prefix. With "console": "integratedTerminal", shows, of course, what you normally would see in the terminal.
    It seems the same behavior in process.stdout.write calls don't show in the console from node2 debug sessions #19750. Ideally vscode can display the typical terminal output in "Debug Console". If it is not possible I think that "outputCapture": "std", or "console": "integratedTerminal", should be added as default.

  2. I suggest to add: "env": {}, too. It makes discover (or remember) how to add env parameters easier for the beginner (and who does not use env variable often).

  3. It looks like that vscode's JSON config already support comments. It would be great to add some comments on what the options are and maybe adding some extra options commented out. The // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 line is great but that link does not seems to have a reference to all the options.

@roblourens roblourens added this to the October 2019 milestone Oct 7, 2019
@roblourens
Copy link
Member

I think we should add the above skipFiles suggestion as a default for all config snippets and dynamically generated configs.

Need to modify here: https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/extension/configurationProvider.ts#L59

and the snippets here: https://github.com/Microsoft/vscode-node-debug/blob/master/package.json#L179

(@connor4312 note that this is in vscode-node-debug, not vscode-node-debug2)

@weinand
Copy link
Contributor

weinand commented Oct 28, 2019

@connor4312 please make sure that there is a test item and an entry in the October 2019 release notes for this.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 25, 2019
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 feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants