Now that we have the v8_inspector available in Node, it would be good to create port of the CLI debugger that uses the new inspector protocol. This would have a few benefits:
Contributions to make this happen would be most welcome.
/cc @pavelfeldman @TheAlphaNerd @nodejs/diagnostics
very much into this idea!
There are quite a few edge cases in the current CLI debugger... I'm not very familiar with the problem space, would the port be fairly trivial or should we consider starting something from scratch (so we can make new and different bugs).
Where is the new protocol documented?
Also does this make sense to open in @nodejs/diagnostics to discuss implementation?
I'd love to help!
The main issue I see is that _debugger.js would need to support WebSocket protocol.
@TheAlphaNerd You can use the protocol viewer for the documentation here: https://chromedevtools.github.io/debugger-protocol-viewer/v8/. For background also see Chrome Debugger Protocol.
The main issue I see is that _debugger.js would need to support WebSocket protocol.
That is true. But since the inspector only uses a minimal subset of WebSockets, I think it should be possible to hack together a small client that is able to talk to the server.
inspector_socket.{h,cpp} only includes server socket implementation, e.g. it does not mask the frames. Also, it does not have JS bindings for the _debugger.js to use.
Of course, that implementation can be extended and the bindings can be introduced.
What do you think of extending CodeGenerator.py to generate a client proxy too? That could be an ideal foundation for the CLI debugger and other clients.
Also finally got to read all the code today :) and have been thinking we might separate:
v8_inspector/platform/inspector_protocol/*v8_inspector/devtools/* andv8_inspector/platform/v8_inspector/public/V8Inspector.[h|cpp] into a separate "Inspector Protocol Gateway" project. That gateway could dynamically load agents like those listed here, and similar to what V8Inspector.cpp does now here, perhaps by looking in a designated folder or a manifest file.
Could be a way to build an ecosystem of domains and agents around this protocol?
Was thinking of working on that next week...
What do you think of extending CodeGenerator.py to generate a client proxy too?
I don't think we should unify to make it a part of CodeGenerator.py. We use a separate 270 lines generator for the DevTools front-end and since the format is well-specified, generator has no maintenance cost. You'll want to generate node-friendly call conventions, use promises, etc.
into a separate "Inspector Protocol Gateway" project
Yes, that sounds like a good direction in general. Some thoughts on that:
platform/inspector_protocol is supposed to be reusable codegen infrastructure for the protocol. You should be able to use it to generate native bindings and wired api for given protocol definition. We use it for v8_inspector and for chromium. It is unclear whether it'll be exposed via v8 API though once we migrate the code there, so we might want to keep it as a third party separately. Not sure yet.
platform/v8_inspector uses the framework above to handle JS-related domains. You wire it to the socket and it does the job. Any other component, including Node could do the same, inspector_protocol should cover all the infrastructure needs. I already extracted js_protocol.json from the giant protocol file and it now belongs to platform/v8_inspector, v8_inspector/devtools no longer exists. We'll roll it along with more fixes soon.
V8Inspector.[h|cpp] is now empty. But you do want a wrapper around the session that would handle the Node-level domains. At this point, inspector_agent is the best place for it.
I already extracted js_protocol.json from the giant protocol file and it now belongs to platform/v8_inspector, v8_inspector/devtools no longer exists. We'll roll it along with more fixes soon.
In #7302.
@pavelfeldman @ofrobots Thanks for the background and the new roll. I'm still planning to work on this soon. Pavel, what you said makes sense and I think we're on the same page; I plan to prototype out the separation somewhere and we can go from there.
https://github.com/nodejs/node/pull/8429 could be as an example / starting point for the CLI debugger port.
Awkward, didn't see this issue when I searched for it. I have a mostly working port here: https://github.com/buggerjs/node-inspect/issues/1
After doing a (more or less) complete implementation of the current debugger interface on top of the new protocol: It's a bit awkward. E.g. given that we now have the inspector command line API and proper remote object support, a "remote repl first" approach (without switching between two modes) using .cmd for step actions etc. might feel a lot nicer.
Where are we at on this?
@jkrems sorry for the late response. It is awesome to see your progress here!
Can you elaborate a bit more on what you consider to be the 'awkward' bits? There a few use-cases, where a CLI based debugger is really useful, e.g. debugging a VM in a production/restricted environment where it isn't possible to expose ports to allow a remote debugger to attach.
Would you be willing to contribute your CLI debugger into node-core?
@ofrobots Sure, to elaborate: I didn't mean "a command line debugger is awkward" but "the current command line debugger user interface is awkward". I suspect it might be a better dev experience if we would default to "it works like the chrome devtools console tab + .bt/.n/... commands for the buttons". So "remote repl first" as opposed to a local repl with magical properties and methods on the context that can turn into a remote repl but loses all debug control commands while the remote repl is active.
There's still some cleanup to be done on the code I linked to (although it should be ~95% there). It should be pretty straight-forward since it's basically a copy of the existing debugger file and has no external dependencies. The main reason I'm hesitant to do it is that I'm not fully convinced that I'd like the end result. ;)
Follow-up: This was discussed in the last diagnostics meeting and it was suggested to move node-inspect into the nodejs org. This work is tracked here: https://github.com/nodejs/diagnostics/issues/67.
node-inspect is now at https://github.com/nodejs/node-inspect. Thank you @jkrems!
Should we close this issue?
I don't see any reason to keep this open given that we have have a new CLI debugger available 7.x already, and #11421 tracks the migration.
Most helpful comment
I'd love to help!