Inspecting Node.js with Chrome DevTools

yury-s commented 9 years ago

Objective

We’ve been thinking about providing unified debugger support across various v8 embedders. One natural approach to that is to reuse Chrome DevTools and provide its JS debugger, performance and memory profiler functionality to Node.js users. What we aim for Node is what we have for Chrome for Android. You basically go to chrome://inspect on a stable version of Chrome and it debugs your running Node instance. You might need to override / specify ports to both if you want them to be non-default. The rest is behind the scenes.

Internal design

DevTools is comprised of the two major components: Frontend providing UI and Backend that instruments inspected application. Frontend is essentially a web application that communicates to the Backend via remote debugging protocol and nothing prevents it from connecting to Node.js instead of Chrome. To support that we’ll need to extract parts of debugger and profiler functionality from Blink into a separate project that would implement DevTools remote debugging protocol. Let’s call the project v8-inspector. Node.js could then expose the protocol to the Frontend over a WebSocket connection.

Proof of concept

I’ve created a prototype demonstrating this approach:

https://github.com/yury-s/v8-inspector - implementation of DevTools protocol for debugger, performance and memory profilers. I started with forking Blink and then removed dependencies that wouldn’t make sense in Node.js. There is definitely more work to be done but I believe this is good enough as a proof of concept. https://github.com/yury-s/io.js/tree/remote-debugger - io.js clone that uses the project decribed above and ws module to allow using DevTools Frontend for debugging/profiling.

Local Node debugging scenario with custom ports would look like this:

Start Node with --remote-debugging-port=9222 command-line flag
Start Chrome with --remote-debugging-targets=localhost:9222 command-line flag
Navigate Chrome to chrome:inspect to see list of inspectable Node.js targets
Longer term

Long-term solution would be to have both Blink and Node.js use the same native v8-inspector library linked against v8. DevTools Frontend development would continue in Blink. For each version of v8-inspector there is a compatible version of DevTools Frontend served from the cloud. This is how it works with Android debugging at the moment: when the user opens DevTools for Chrome running on Android, the desktop browser loads a version of DevTools UI corresponding to the version of Chrome running on Android and forwards all traffic from the device to that Frontend.

Open questions

Before moving forward with this approach and investing into Blink refactorings necessary for extracting common code into its own library we’d like to collect some feedback on whether Node.js community would be interested in that. In particular, is there an interest in integrating such debugger library into Node.js core?

This will require compiling Node with the v8-inspector library and providing Node.js-specific implementation of interfaces required by the debugger.
Remote debugging protocol will be exposed to third party Node.js modules as a simple API similar to what is available for Chrome extensions (https://developer.chrome.com/extensions/debugger). Client of such API should run on a separate thread not to interfere with JavaScript in the inspected Node application. In the prototype all debugger events are forwarded from the main thread to a node instance running on a separate thread.
Implementing transport for the protocol. It can be a module that would use the debug API described above and send protocol commands over WebSocket. E.g. in the prototype it is a simple web server based on ‘ws’ module: https://github.com/yury-s/io.js/blob/remote-debugger/lib/remote_debugging_server.js. Bundling this module with the core would enable zero-conf rich devtools experience (on par with Chrome’s DevTools).
As the project evolves it will require adding new instrumentation to use some new features such as asynchronous call stacks. On the DevTools side we’ll be providing generic API that would work for both Blink and Node.js
There is some maintenance cost related to the debugger if Node.js decides to ship with it but I don’t expect it to be much higher than for the current debugger that includes debugger-agent.cc. Ideally, v8-inspector would be included as part of the v8 checkout and the obsolete debugging protocol implemented by v8 would be removed, but that is not a done deal yet.

cjihrig commented 8 years ago

Bring in websockets (ws) as an internal module to avoid exposing it to user space.

This was discussed a while ago. I don't remember what the final verdict was. Would we be taking ws wholesale in the deps folder, using it as the baseline for a core implementation, starting a new implementation from scratch, something else?

rvagg commented 8 years ago

@ofrobots:

Even though the window for getting this in into V8 5.0 is closing, we do still have a runway for Node.js 6.0.

My recollection of the outcome from https://github.com/nodejs/LTS/issues/62 was that there was support for being flexible with upgrading V8 in Node.js v6 if we can do it in a way that doesn't break ABI for addons as this is the primary concern that has lead to our V8-locking policy in the first place. For instance, the 4.8 changes have turned out to be pretty minimal from an API perspective and if we had have had this policy in place for earlier then Node.js v5 may have had an upgrade from 4.7 to 4.8 at some point. I'm going to have to revisit the CTC meeting where we discussed this and write up some proper minutes for that discussion because Chris was taking notes and had to leave so our minutes doc is incomplete. I believe that we agreed that the overriding concern was ABI breakage and then on top of that stability prior to cutting LTS, so there would have to be a buffer period before October where we rule out upgrading even if it's possible simply to create stability and predictability for users as they start testing v6 for LTS adoption.

So, if the worldview that I have in my head is an accurate reflection of the general agreement then that would mean that V8 5.0 isn't necessarily the final word for Node.js v6 LTS, we could have a path to 5.1 and even maybe 5.2 during the v6 stable period. So that would suggest that it may not be quite as difficult to get some form of this into v6 LTS.

@cjihrig relevant discussion on websocket support is here: https://github.com/nodejs/node/blob/master/doc/tsc-meetings/2015-09-02.md#inspecting-nodejs-with-chrome-devtools-2546

That, and my recollection goes something like this: We're open to adding websockets support purely for supporting this and we were much more happy with the idea of doing it privately just for this to stop us jumping on an API we may not be happy supporting in core although there is a nice path to exposing such an API if we really wanted to. wrt ws specifically, I don't believe we ever went there in the TSC/CTC level and assumed we'd be implementing our own. There is some existing discussion here: https://github.com/nodejs/node/pull/4842 leading to an EP here: https://github.com/nodejs/node-eps/pull/6. My reading of this is that there is very little enthusiasm for going this route, but perhaps if it gives us a kick-start on getting this problem solved we should be open to it—i.e. afaik there are no decisions here so it's all up for discussion.

benjamingr commented 8 years ago

I just want to bring up the option of exposing the hooks required for a userland websockets library (or http library) to perform the necessary instrumentation rather than core endorsing ws or another specific module.

In general - I'm personally much more in favor of exposing more hooks for the debugger and not endorsing large modules that have to be maintained and de-facto enforce an opinion in Node.

I'm not sure how feasible that is but I think it should be discussed.

bnoordhuis commented 8 years ago

@benjamingr There's no endorsement, 'ws' won't be exposed to user code. It'd be a private implementation detail that can be replaced anytime.

Hooks or overrides on the other hand means committing to an API and supporting it over longer periods of times, possibly indefinitely. It gains us nothing that I can see, it would just make maintenance more of a hassle.

cjihrig commented 8 years ago

Is this something that the CTC needs to revisit?

benjamingr commented 8 years ago

@bnoordhuis hooks would mean userland libraries could use the instrumentation to provide very nice debugging tools. For example - if an HTTP2 library or a library for another protocol (IRC or FTP for example) could use the network panel in Chrome to track requests in a way that provides developers insights it might be interesting. Node is not only browser protocols like HTTP and WS.

That said, those two working is a lot better than a solution in which nothing works.

jkrems commented 8 years ago

@benjamingr If I understand it correctly, it's not about instrumenting ws so it shows up in the network panel. The devtools use web sockets to communicate with the node process and for that purpose ws would be used. Making web sockets used by the debugged app show up in the network tab is out of scope for all of this afaict.

ofrobots commented 8 years ago

@rvagg: The discussion in nodejs/LTS#62 led to a possibility of being able to upgrade V8 in Node.js v6 if the ABI differences could be bridged. Still, I think it is a good idea to bring v8-inspector into Node.js as a direct dependency for now. The work on integration with Node.js can begin, on the Node.js repository, with more feedback and collaboration from the community. Downstream vendors and module authors could potentially start building debug tooling built on top on the new debug protocol.

Down the road, if we do end up picking up V8 5.1 or 5.2 for Node.js v6, we can simply drop the direct dependency on v8-inspector. I am afraid that waiting until then would reduce the runway to collaborate on the integration and get it polished in time for the v6 LTS.

ChALkeR commented 8 years ago

Bring in websockets (ws) as an internal module to avoid exposing it to user space.

Do we absolutely need it? Are there any chances that we could move most of the code in question here to module outside of the core (i.e. that one has to explicitly npm install)?

Including another huge chunk of thirdparty code into core gives me an uneasy feeling. Even if it is not exposed, it will have to be supported by the collaborators here. For example, any security release in ws (there was one not so long ago, for example) would result in security releases of all Node.js branches that get this feature, and the same for all other modules that would be pulled in. That would also potentially break the schedule and could even force us to introduce some backwards incompatible changes.

For example, would an update of the DevTools that would shrink the range of supported Chrome/Chromium versions be treated as semver-major? What if it includes a security fix? There are more questions like those.

As for the hooks — even introducing a private-ish hook api that would somehow check that it's being used only by a specific module (and moving all the functionality into that module) seems like a more acceptable solution to me.

jkrems commented 8 years ago

@ChALkeR I guess any websocket implementation would do, including a simplified version of ws. I wouldn't expect a manual npm install being the final solution. I'd assume that even if ws would be picked, it would end up in deps/ including all of its dependencies.

ChALkeR commented 8 years ago

@jkrems I suppose that everything I noted above is still valid with the websocket implementation being in ./deps/.

jkrems commented 8 years ago

@ChALkeR Sorry, I completely mis-parsed your first paragraph. Thought you were talking about the fact that it currently does require an explicit npm install when setting up (https://github.com/repenaxa/node/commit/86939cb60b22066f4ee6e818ef879c3dbd6282aa#diff-04c6e90faac2675aa89e2176d2eec7d8R21). But now re-reading your comment it sounds like you would prefer it if node would just ship with the hooks for the protocol and leaving it up to an external (possible npm-) module to actually hook it up to a websocket?

ChALkeR commented 8 years ago

@jkrems Yes, that's what I meant.

ofrobots commented 8 years ago

@ChALkeR: The problem with having part of the functionality being provided by user-space module is that it breaks the out-of-the-box debug experience. In many cases, at the time the user realizes that need to debug something, it may already be too late to get an npm module installed.

That would also potentially break the schedule and could even force us to introduce some backwards incompatible changes.

I am not sure I follow the point about backwards incompatible changes – since it is not going to be externally exposed. We (node-core) will need to address security vulnerabilities in upstream ws (or alternative websockets implementation) and dependent packages. In my opinion, the security impact would be higher if core functionality (remote debug server) were to be dependent on externally provided modules that node-core cannot patch by providing a security update.

I think these are excellent discussions. I suspect that this thread will get unwieldy fast if we were to have these discussions here. Apropos https://github.com/nodejs/node/issues/2546#issuecomment-190301820, I propose that we create a branch to work on the v8-inspector integration and use individual PRs to discuss individual design points (websockets, event loop integration, ergonomics, etc.). If there are no objections, I'll proceed with that route.

ChALkeR commented 8 years ago

@ofrobots

I am not sure I follow the point about backwards incompatible changes – since it is not going to be externally exposed.

The debugger is going to be exposed, and it could lose compatibility with older browser versions at some point. Or am I misunderstanding something?

In my opinion, the security impact would be higher if core functionality (remote debug server) were to be dependent on externally provided modules that node-core cannot patch by providing a security update.

I meant — package it separately (perhaps installable with npm install), but keep in the nodejs org. Like nan, for example.

Btw, that means that we could force people to upgrade that tool if we really need them to — Node.js could check the version of the debugger tool being used and refuse to work with it if it's too old.

jkrems commented 8 years ago

@ChALkeR Given that this interface is - afaik - meant as a replacement for the current JSON/TCP API, putting the interface into something external to node would mean that it would no longer be possible for something like an IDE to run a node program in debug mode without having to "patch in" that library. That doesn't sound like a great developer experience imo.

trevnorris commented 8 years ago

Only skimmed over the recent comments, so excuse any duplicates.

The debugger will require the websocket server to run off the main thread, IIRC. Along those lines, it would be more beneficial if that implementation was done w/o requiring a v8 stack. Which rules out using a third-party module.

ofrobots commented 8 years ago

@trevnorris

The debugger will require the websocket server to run off the main thread, IIRC. Along those lines, it would be more beneficial if that implementation was done w/o requiring a v8 stack. Which rules out using a third-party module.

Indeed. That's another option we are investigating. More specifically, we're looking at what would be involved in pulling out the websockets implementation devtools is using in blink. We haven't looked at it enough to know in detail whether we can extract it and plug it into Node. Note however, that we are more interested in getting devtools working with Node than we are in designing a websockets implementation for core. We're trying to find the path of least resistance here.

@ChALkeR

Btw, that means that we could force people to upgrade that tool if we really need them to — Node.js could check the version of the debugger tool being used and refuse to work with it if it's too old.

I think this stance is hostile to our users. If we think debug functionality is important to core, then we will need to own up to the security implications of supporting such functionality. There are cases where the user trying to debug may not be able to install a module from npm (e.g. shrink wrapped deps scenarios, etc.) Having said that, we are also looking at alternative (non-js) implementations of websockets (collaboration from the community on this is welcome). It is possible that we may be able to avoid this debate.

jkrems commented 8 years ago

I'd volunteer to port the needed parts of ws to C++ but I'm not sure if that would be a net positive long-term. Those parts would need some level of maintenance and I'm assuming that bigger chunks of C++ will always be scarier to contributors than JS. Is the current solution (additional V8 context for the debugger thread) causing known issues? There's some ugly parts (e.g. src/node.js having to do some branching if it's running as a debugger thread) but otherwise it seems - relatively - fine to me.

Maybe (random thought) the code for the devtools bindings could even live in the exact same place the current v8 protocol code lives. This might help make the branching between the two interfaces less confusing.

ChALkeR commented 8 years ago

@trevnorris

The debugger will require the websocket server to run off the main thread, IIRC. Along those lines, it would be more beneficial if that implementation was done w/o requiring a v8 stack. Which rules out using a third-party module.

Adding in a brand new c++ implementation of WebSockets gives me an even more uneasy feeling.

I might be missing something again, but why creating an IPC interface on the main thread and hooking to it and passing over WebSocket by a standalone process (that could reuse ws) is not an option?

YurySolovyov commented 8 years ago

There is node-lws which I think is fully C++

joshgav commented 8 years ago

@ofrobots (emphasis added)

To this end we have been working on extracting the DevTools backend from blink, extricate all its dependencies on Blink, and get it integrated into V8 instead.

Once extricated from Blink, would it make more sense to permanently keep v8-inspector as an independent component from v8? That would give us (Node) more flexibility as you already noted, and better facilitate external contributions of additional functionality to v8-inspector.

ofrobots commented 8 years ago

@joshgav The end goal is to get v8-inspector integrated directly into V8 as the fully featured debug API for V8. It is very closely tied to the internals of V8, and keeping them separate is going to be more work for embedders and the developers alike. We would love to see contributions from the community – I think contributing would be a lot easier once this is in V8, but if there is friction please do let us know. /cc @repenaxa.

pavelfeldman commented 8 years ago

Although v8-inspector will live outside of Blink, it will still be used there. As well as in other non-Chrome and non-Node V8 embedders. We were thinking about making it a third party dependency, but eventually decided against it:

it creates a new step in the dependency roll: Blink -> v8-inspector -> v8 that we can not afford
it limits our ability to iterate due to two-sided patches and the v8.h surface limitation
v8-inspector exposes the debugging APIs (wired and strongly-typed) that needs to be versioned and introducing an dependent component with a separate API versioning sounded like too much overhead.

Having said that, we plan to move v8-inspector into v8 along with its contributors! So it would start with the Chrome DevTools and some V8 people and will grow from there. As @ofrobots mentioned, we can work on minimizing the associated friction.

ofrobots commented 8 years ago

I just opened a PR with initial v8_inpector support: https://github.com/nodejs/node/pull/6792.

trevnorris commented 8 years ago

Can this be closed now?

cjihrig commented 8 years ago

I think so. Please reopen if I'm wrong.

xaxxon commented 8 years ago

What is the current status on the effort to integrate in with vanilla v8? I'm currently building this functionality and have it minimally functional but would rather not, but I don't know how to find out about the progress being made.

Thank you.

bmeck commented 8 years ago

@xaxxon use node --inspect on v6+, should work fairly well

xaxxon commented 8 years ago

I'm not using node, though.

On Wed, Oct 5, 2016 at 8:23 AM, Bradley Meck notifications@github.com wrote:

@xaxxon https://github.com/xaxxon use node --inspect on v6+, should work fairly well

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/2546#issuecomment-251707069, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIycuUi5Tj8B7sVus9suliVTUI_stEeks5qw8DWgaJpZM4FyAmT .

cjihrig commented 8 years ago

You might want to check the V8 issue tracker then.

eugeneo commented 8 years ago

The code is in the V8 now, e.g. - https://github.com/v8/v8/tree/ba41697cbd72ea7dcdd89e6dbe9090ecb156ce6c/src/inspector

On Wed, Oct 5, 2016 at 8:24 AM Colin Ihrig notifications@github.com wrote:

You might want to check the V8 issue tracker then.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/2546#issuecomment-251707826, or mute the thread https://github.com/notifications/unsubscribe-auth/AARkrVMxf-2hCea0SHccmwmrYQR7_m7Wks5qw8E8gaJpZM4FyAmT .

xaxxon commented 8 years ago

thank you.

On Wed, Oct 5, 2016 at 9:24 AM, Eugene Ostroukhov notifications@github.com wrote:

The code is in the V8 now, e.g. - https://github.com/v8/v8/tree/ba41697cbd72ea7dcdd89e6dbe9090 ecb156ce6c/src/inspector

On Wed, Oct 5, 2016 at 8:24 AM Colin Ihrig notifications@github.com wrote:

You might want to check the V8 issue tracker then.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/2546#issuecomment-251707826, or mute the thread https://github.com/notifications/unsubscribe-auth/AARkrVMxf- 2hCea0SHccmwmrYQR7_m7Wks5qw8E8gaJpZM4FyAmT

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/2546#issuecomment-251724922, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIyckAW2RICpAYxAYi6zBOjV2PykixBks5qw88-gaJpZM4FyAmT .

msaspence commented 7 years ago

I wonder if it is currently or would be possible to access this url programatically from the Node process?

eugeneo commented 7 years ago

No, not yet. We can expose it if it is really needed.

msaspence commented 7 years ago

I'm just think it would be nice to be able to expose it so in the process can open the tab for you automatically if you so wish. However It seems that you can actually determine the URL yourself if you know the port, which you can specify with --inspect=1234

eugeneo commented 7 years ago

UUID is not exposed.

On Wed, Nov 2, 2016 at 9:55 AM Matthew Spence notifications@github.com wrote:

I'm just think it would be nice to be able to expose it so in the process can open the tab for you automatically if you so wish. However It seems that you can actually determine the URL yourself if you know the port, which you can specify with --inspect=1234

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/2546#issuecomment-257927247, or mute the thread https://github.com/notifications/unsubscribe-auth/AARkrVmlK-gwGRcNbq4PZSlaTl8O2cSEks5q6MCRgaJpZM4FyAmT .

msaspence commented 7 years ago

With chrome-cli installed (open doesn't like the chrome-devtools protocol) I have the following proof of concept working on node 6.9.1

const { exec } = require('child_process');
  exec('chrome-cli list links', (_error, stdout) => {
    const url = `chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=localhost:${process.env.npm_package_config_debugPort || 9221}/node`;
    const match = stdout.match(new RegExp(`\\[(\\d+)\\] chrome-devtools://devtools/bundled/inspector.html\\?experiments=true&v8only=true&ws=localhost:${process.env.npm_package_config_debugPort || 9221}/node`));
    const tabId = match ? ` -t ${match[1]}` : '';
    exec(`chrome-cli open "${url}"${tabId}`);
  });

Where $npm_package_config_debugPort is provided to the node --inspect flag.

june07 commented 7 years ago

I was having the same issue a few days ago and wrote a Chrome extension to solve it. Would love any feedback.

http://june07.com/nim

Direct Chrome Web Store link: https://chrome.google.com/webstore/detail/nim-node-inspector-monito/gnhhdgbaldcilmgcpfddgdbkhjohddkj

nodejs / node