Update inspector ready message to be vendor agnostic

[auchenberg]

Checklist

• tests and code linting passes
• a test and/or benchmark is included
• documentation is changed or added
• the commit message follows commit guidelines

Affected core subsystem(s)

v8inspector

Description of change

Per concern raised in Add v8_inspector support, this PR updates the inspector ready message to be vendor agnostic, and removes the Chrome DevTools specific URL, that requires Node users to install and use Chrome as the preferred debugging tool.

Principle behind

The principle for this change, is that Node as a runtime, shouldn't favor one debugging tool or vendor, but the --inspect flag should simply expose a generic debugging endpoint that's discoverable by tools such as Chrome DevTools, Node-inspector, VS Code and others.

It's a fact that Chrome is cross platform browser and could be installed for potentially all node users, but the same argument can be used for other debugging tools such as VS Code, Sublime and others, and those tools aren't being favored by Node, so neither should Chrome DevTools.

If the argument is the convenience, this could just be achieved by Google or any other vendor providing a CLI that launches node --inspect and launches the specific tool connecting to the specific node process.

Think node-chrome-debug index.js -> node --inspect index.js + opens chrome-devtools://devtools/ in Chrome. Collaboration with the node-inspector could be an ideal candidate for this.

(Ping @boneskull, @ofrobots, @pavelfeldman, @joshgav, @rvagg)

[MylesBorins]

I like the idea behind this change, but it seems that the only information this will give individuals is the port that the process is running on. Without the specific link it is not easy / intuitive to get a debugging session running.

When this feature is officially supported it will definitely be worthwhile to have a more vendor neutral link printed... but as it is this change seems like it will only make things more difficult for individuals.

to be fair this pain can be solved through documentation, but for now the decision has been to leave the feature undocumented while it is unstable

[auchenberg]

@thealphanerd The console message could easily be updated to output the HTTP JSON endpoint http://localhost:5858/json if the lack of debugging information is a concern.

I see this discussion to be more of a principle one: Whether the Node community want to prefer one tooling vendor over another, and ultimately what provides the best debugging experience for Node users.

[MylesBorins]

@auchenberg that still makes an extra step for people to get up and running with the debugger today, and it isn't exactly intuitive.

There are rumblings right now in nodejs/diagnostics#46 about expanding the scope of the wg to a diagnostics WG that would include debugging under its umbrella.

I highly suggest chiming in there and joining the discussion. We are trying to get together as many stakeholders from various organizations to make sure that all concerns (such as this one) are being thought about and considered.

FWIW I completely agree that it should be vendor neutral when it is officially supported, I just think that the approach in this PR reduces usability and I'm unsure how we would properly let me know how to use the feature otherwise

[joshgav]

If I may be so bold, I suggest we agree that the text should be updated to be more vendor-agnostic prior to including v8_inspector and inspector_agent in a release, though not necessarily immediately.

In fact, if the only helpful instructions require something specific to Chrome DevTools, that's likely an indicator that we don't yet have enough ecosystem support to ship.

@ofrobots, can you all think about this too? We should strive for instructions which are short yet helpful and apply to all tools equally.

[MylesBorins]

@joshgav one potential solution would be an "unofficial" guide, perhaps even a wiki. A place where each vendor can keep up to date instructions on how to use the debugger with their debugging tools.

Perhaps something in the same vein as https://nodejs.org/en/docs/es6/

edit: the idea being that the print out can post to that resource

[joshgav]

@thealphanerd makes sense, perhaps we should start that in the debugging section of nodejs/tracing-wg after merging nodejs/diagnostics#47?

[evanlucas]

So we would now have to "figure out" the url to use with chrome? I feel like this makes it a lot less user friendly at this point...

I am -1 on the pr as it stands now

[auchenberg]

@thealphanerd Indeed change makes it less intuitive from developers to debug node, as it decouples the new Chrome Debugging endpoint provided by v8_inspector, from a specific front-end, Chrome DevTools, and brings back Node to be vendor natural.

The suggestion is that Node core should be kept vendor agnostic, and each vendor should be providing a CLI or similar that integrates wide Node to provide an intuitive experience. Such CLI is suggested above and could easily be built. These front-end specific integrations could easily be listed in a Wiki page or similar.

Before:
node --inspect

After:

npm install node-chrome-debug -g

ncd index.js
To start debugging, open the following URL in Chrome:
chrome-devtools://devtools/remote/serve_file/@521e5b7e2b7cc66b4006a8a54cb9c4e57494a5ef/inspector.html

[cjihrig]

I made a really simple Chrome extension that opens Chrome's DevTools with V8 Inspector, given only the port as input - https://github.com/continuationlabs/node-v8-inspector

Is that a route we could explore?

[jasonLaster]

Thanks @auchenberg.

+1 for vendor agnostic

There are many potential node debugging front ends. Mozilla is making debugging node a first class citizen of the new Firefox debugger.

[bnoordhuis]

-1, decreases usability and discoverability. An acceptable change might be printing a second line that says "If you are using a different debugger, connect to this address: address:port".

[rvagg]

We don't exactly have a plethora of tools that can talk this yet right? Until we get to the point where people can make a choice of "preferred debugging tool" that isn't just DevTools then -1 from me and we can revisit this a little later. For now the priority is to get people using this so we can perfect it.

[auchenberg]

@thealphanerd @joshgav Did you move forward with the vendor agnostic tooling and CDP discussion in the new Diagnostics WG?

[auchenberg]

@rvagg Today the community already have debuggers from Microsoft (in VS Code), and Jetbrains (in WebStorm) that are compatible with the new CDP endpoint exposed by v8_inspector, so there's already a tooling choice today.

@bnoordhuis @evanlucas The assumption of usability is quite biased, as it assumes the developer already have Chrome installed on their computer. Discoverability should be solved within Chrome, just like it's solved within other debugging tools such as VS Code today.

If we there was a PR submitted that changed the inspector ready message to contain a Visual Studio Code specific URL that required VS Code to be installed (available and open sourced on all platforms like Chrome), would such a change be accepted? What tooling front-end should be favored?

The existing V8 protocol endpoint doesn't favor one tool over other tools, and I don't see why v8_inspector should change that. The existing debugging message Node doesn't contain specific links to on how VS Code, JetBrains and other V8 compatible tools should be started, that's handled by community projects like node-inspector, and going forward my point is that Node core should be kept that way.

[evanlucas]

@auchenberg can you point me to some documentation on how to utilize the debugger exposed by v8_inspector in vscode? I have not used it and feel that using it could help me understand your side a little better. Thanks!

[auchenberg]

@evanlucas See https://github.com/auchenberg/node-v8-inspector-vscode-sample, and notice 3rd party tools are currently blocked by #7227

[evanlucas]

@auchenberg thanks

[joshgav]

@auchenberg This is a good conversation to bring to nodejs/diagnostics now; see nodejs/diagnostics#52 for a related issue. It would also be great to add a page in that repo describing how to use VS Code with v8_inspector; maybe @jasonLaster can do the same for Firefox Debug Tools.

I think the key point in this conversation is that the hint message for --inspect or --debug should be generic before we can completely endorse v8_inspector and inspector_agent by adding them in a release. Does anyone disagree with that?

[rvagg]

thanks for the info @auchenberg, useful to know.

I think the key point in this conversation is that the hint message for --inspect or --debug should be generic before we can completely endorse v8_inspector and inspector_agent by adding them in a release.

yes, +1 to this

[rvagg]

Do we have anyone from Jetbrains involved in the diagnostics group? Does anyone have a contact there we could reach out to?

[joshgav]

@rvagg > Do we have anyone from Jetbrains

I'm working on this starting with an indirect contact.

[pavelfeldman]

/cc @denofevil for WebStorm

[bnoordhuis]

I think the key point in this conversation is that the hint message for --inspect or --debug should be generic before we can completely endorse v8_inspector and inspector_agent by adding them in a release. Does anyone disagree with that?

I do. I'm not convinced by the arguments made so far. Also:

An acceptable change might be printing a second line that says "If you are using a different debugger, connect to this address: address:port".

Problem solved, no?

[denofevil]

Thanks, @pavelfeldman!
The current status is the following: we’re starting to investigate the integration and hopefully @develar will be able to give an update on how we can use the debugger exposed by v8_inspector later today.
We’re here on the same page with @auchenberg.

[develar]

update: I missed --inspect=9229 --debug-brk options.

I don't have any objections to current message. Because:

• if IDE is used, message means nothing because IDE starts node, not user.
• if user want to start remote debug (i.e. start on one machine and debug from another) — node in any case cannot listen 0.0.0.0 and custom vendor tool is required (it is another bug).
• if user doesn't use IDE, maybe it is more user-friendly to print short usage guide — e.g. I start node --inspect and what next? (if we want to avoid RTFM issues).

How IDE debugger works: * find a free port * trying to listen port (until it is not available) * start process and specify this port (`--debug-brk=port_number`) * process doesn't execute anything until debug client is not connected. Current implementation `--inspect` is not suitable to debug not server-like apps or debug server bootstrap. I think, new protocol should be implemented in the same way as current `--debug-brk`. Should be a way to specify port and "break on first line" behaviour. As @auchenberg said, > each vendor should be providing a CLI or similar that integrates wide Node to provide an intuitive experience yes, current limitations can be solved using vendor-specific bootstrap module. But I think it is a wrong way — there are no valid reasons to reinvent the wheel and complicate user setup / vendor tool (you want just start node in the docker container — if vendor tool will be required to start it properly, it will be a nightmare for all users). Excellent illustration is a MS [RemoteDebug](https://github.com/Microsoft/nodejstools/blob/master/Nodejs/Product/Nodejs/Debugger/RemoteDebug/RemoteDebug.js) — instead of fixing nodejs issue (no way to listen 0.0.0.0), custom vendor tool is introduced and users have to somehow deal with it. How JetBrains debugger works for Google Chrome? We load blank page at first, set breakpoints and only then load real user page (using special Chrome extension). I really want and hope that implementation in the nodejs will be more IDE-friendly and custom vendor tool will be not required.

[pavelfeldman]

-1 from @pavelfeldman, Google

I was always saying that debugging ergonomics concerns were up to the Node community.

[bnoordhuis]

My personal feeling is that the current console message has been snug in by the Chrome team, and is being used as a political instrument to move Node closer towards Google's tooling ecosystem.

I don't feel that way at all. I was the main reviewer for the inspector pull request and I work for IBM.

[auchenberg]

Note the lack of my opinion on this issue. I am leaving it to the community to decide how to proceed. I do find this suggestion of sinister motives distasteful and offensive. I think your comment is bringing politics into this issue that had been technical so far.

First, I'm sorry you find my comment offensive as that wasn't my intention. Secondly I'm raising this concern exactly to let the community decide as the question of a specific tooling recommendation is more of a strategic/political decision than technical.

The technical part of adding the Chrome Debugging protocol to Node had landed, and it's wonderful for the broader tooling ecosystem, and I'm sure everyone involved in this debate highly appreciates the efforts Google has put into making this happen. I do at least!

The above concerns has been raised, and in the above comment I've tried to provide an overview of the current opinions and pro/cons.

I'm unsure on how concerns like these are addressed by the Node community and how the decision process for such things is. I've stated the concerns of recommending one specific tool directly from Node, and the suggestion is to keep Node vendor neutral, as pre-v8_inspector.

[rvagg]

@auchenberg I think we should just apply some patience here, we have some runway here because it's still in experimental form. I'm pretty confident that by the time we're ready to call it an official feature of Node.js that we may have broad enough agreement to update the message to be at least a little more agnostic. For now it reflects an easy path for users to get up and running with this feature and providing feedback for us.

I'm tagging this PR as part of the 7.0.0 milestone as it may tie in well with the decision whether to move forward with this into v6 LTS or not. How about we just let this brew for a while and our understanding and feelings about this feature will evolve.

[jasnell]

Ping...is this still something we want for v7?

[joshgav]

Thanks for the ping @jasnell. To best reflect our intended open, vendor-agnostic platform, I think we should figure out how to make the message helpful and immediately actionable without referencing Chrome/V8-specific details.

Will review this thread and think about next week and round up a suggestion or two from our crew here at MS.

[jasnell]

@joshgav ... just a heads up that I'm planning to cut the v7.x branch on Monday. This change doesn't need to land before then but it would be good to have an idea of which way this is heading.

[Fishrock123]

From the CTC meetining sounded like @joshgav had a update for here.

Are we still on track / aiming for v7 for this one?

[joshgav]

We arrived at three options in the Diag WG meeting (notes here):

1. Provide short instructions when --inspect is specified for using several different tools (e.g. Chrome DevTools, VS Code, WebStorm).
2. Provide a URL to a page with more detailed instructions, and specify a generic ws URL, e.g. ws://localhost:9229.
3. Use a standard scheme (i.e. the part before '://' in a URL) and mechanism for discovering and connecting to listening processes, and list that URL.

We settled on 2 for now as the easiest to implement, with 3 as a long-term goal.

FWIW, this isn't blocking for v7 and won't be a breaking change in the future, but it would be nice to have it updated before release. I hope to write up the initial page in the next few days...

/cc @nodejs/diagnostics

[danwkennedy]

/shamelessplug

Since this is going to make it a little more difficult to start a debugging session, I made a chrome extension that lets you save configured debuggers and will even tell you if they're online. That way I don't have to copy + paste a bunch of urls all the time. We also use Node in a microservice + docker ecosystem so we need to keep track of a bunch of preconfigured ports everywhere.

The link is here:
https://chrome.google.com/webstore/detail/node-debugger/onmjjpakkhjdbkmpoocapmhnmaedeipb

[joshgav]

@danwkennedy cool! Can you point us to the source for that extension?

BTW - I think we should close this issue now in deference to the fresh start at #8978

[danwkennedy]

@joshgav: The source for the extension is here: https://github.com/danwkennedy/node-debugger

I'm also working on a longer blog post that will explain our workflow and how the extension fits into it. That'll take a little longer, though.

[jasnell]

Checking in on this. We're a week out from the v7 release. This would need to be rebased and updated this week in order to land for that.

[joshgav]

@jasnell - this conversation has continued in #8978 so closing this in deference to that newer PR.

In the newer PR we haven't reached consensus on a generic hint text yet unfortunately 😢

[jasnell]

ok. thanks for the heads up