Support Network Inspection

eugeneo commented 7 years ago

Network domain is what Chrome DevTools (and any other interested frontends) use for inspecting the HTTP and WS traffic in the browser. This document provides some insight into Chrome DevTools UI and here is a description of the protocol for obtaining that data.

We propose to introduce a Node specific agent that would expose the same (or subset) of this protocol to enable the UI. It does not seem possible to share the implementation between Blink and Node.js.

This agent would also introduce an API for the modules to report the events. E.g. WebSocket modules should be able to notify the agent when WS frames are sent or received.

sidorares commented 7 years ago

would be great to have support for node tcp/udp as well, not sure if that can fit into existing UI and debugger protocol

jkrems commented 7 years ago

Would it make sense to use trace events as the interface to the agent? That would also provide an interface for 3rd party modules (e.g. Websocket) out-of-the-box.

I'm not sure if tcp/udp would become confusing because it's another level than the others. Would http requests show up twice effectively? But having a story for things like memcached's TCP protocol would be great. The obvious/hacky solution would be for them to pretend to be websocket frames.

eugeneo commented 7 years ago

Looks like trace events might be usable for this. HTTP requests/responses might get quite big - I wonder if there's any limit on event sizes...

+@matthewloring, @ofrobots - would it be possible to use the trace events framework for this? E.g. inspector would have an agent that "listens" on event categories and then sends inspector protocol notifications.

jkrems commented 7 years ago

One thing I ran into with a similar feature in bugger: Retaining all request and response bodies in the process can become problematic, especially when there's no easy way to disable it. E.g. "hey let me profile this app while sending a couple of thousand of requests through it to find that memory leak" - aaand you're running out of memory because all request/response bodies are being kept in memory by the agent.

matthewloring commented 7 years ago

There is not currently a mechanism for observing trace events as they are generated. We could explore this in the future once the design in https://github.com/nodejs/node/pull/9304 solidifies.

alnorris commented 7 years ago

Would it be possible to monkey patch this? Similar to how node inspector does it?

paulirish commented 7 years ago

@alnorris honestly it looks like the existing monkeypatch from node-inspector (Injections/NetworkAgent.js) would get pretty far.

The emitted events would just need to make their way to the InspectorAgent

joshgav commented 7 years ago

My architecture proposal in https://github.com/nodejs/node/issues/7393 is largely about allowing implementation of Domain.Method dispatchers other than those tied directly to V8, which is all we have now. The Network domain implementation we're discussing in this thread could be handled by just such an alternate dispatcher if our architecture allowed it.

Allowing alternate dispatchers would a) allow complete alternate implementations, e.g. for other runtimes like Chakra or SpiderMonkey; and b) facilitate one-off (monkey-patched?) alternate or additional implementations for specific Domains and Methods.

/cc @kfarnung who's been looking into such a rearchitecture and Inspector's impact on e.g. Node-ChakraCore more deeply.

joshgav commented 7 years ago

Perhaps a way to facilitate inspector "addons" would be to update NodeInspectorClient::dispatchMessageFromFrontend to route based on the Domain name. That is, Debugger, Profiler, Runtime, and all the domains handled by V8 would be routed to V8Inspector as they are now; and others like Network could be routed elsewhere.

joshgav commented 7 years ago

@eugeneo I changed the title so it would be more obvious what this issue is about, please change back if you disagree. Thanks!

eugeneo commented 7 years ago

@joshgav I agree. I want to look into JS handlers for the inspector messages. Currently I am waiting for the inspector JS API PR to land.

jkrems commented 7 years ago

Was there any movement on this? I think it would be a really great feature. It looks like in Chrome there's a pretty straight-forward check (InspectorSession::DispatchProtocolMessage):

void InspectorSession::DispatchProtocolMessage(const String& method,
                                               const String& message) {
  DCHECK(!disposed_);
  if (v8_inspector::V8InspectorSession::canDispatchMethod(
          ToV8InspectorStringView(method))) {
    v8_session_->dispatchProtocolMessage(ToV8InspectorStringView(message));
  } else {
    inspector_backend_dispatcher_->dispatch(
        protocol::StringUtil::parseJSON(message));
  }
}

We could "just" introduce an extension to the existing inspector JS API that allows to register additional handlers. Or, alternatively, a C++ API for the same (might be less confusing when it comes to execution / threads).

The idea here is that we could start with opening up an experimental low-level API and then later decide if and what kind of node-specific dispatchers we'd want.

eugeneo commented 7 years ago

Right now I am working on custom protocol handlers on the Node side. I am implementing "Target" domain (that should allow debugging child processes). "Network" domain can be implemented afterwards, but it would require effort from module writers - e.g. WebSocket implementors will need to specifically report WS frames.

eugeneo commented 7 years ago

@jkrems - this is the Node counterpart - https://github.com/eugeneo/node/blob/df8e41d3be705410e3a3389b8660b3289e25399f/src/inspector_agent.cc#L255

Minor difference is that I am passing unrecognized messages to V8 dispatcher to reuse "method not found" notification.

kaycebasques commented 7 years ago

Chrome DevTools tech writer here. Just talked to somebody who's using node --inspect to debug his Node server via DevTools. He was looking for the Network panel, and was confused why it was missing. His goal was to debug the network requests that his server was making. Seems like a reasonable use case to me!

nmiddendorff commented 6 years ago

Any update?

Kielan commented 6 years ago

Many of us would love to know, there are two medium articles which did not age particularly well and beyond that not much of a page on the internet to explain YES THIS is how you inspect http requests through node with the same powers as chrome network inspector tab.

this nodejs page in the docs for example references cancelled or unsupported projects for the last 2-3 versions of node https://nodejs.org/en/docs/guides/debugging-getting-started/

How can we users help ensure there is a place on the internet for users to find timely documentation on inspection of http requests and options.

mcollina commented 6 years ago

@Kielan thanks for weighting in! I don't think it's currently possible to see the HTTP requests in the inspector, so there is no place where this is documented. Possibly, we should document that no, it is not possible to inspect http requests and responses. However, work is being carried on to enable this.

Considering that we can now route trace_events into the inspector (https://github.com/nodejs/node/pull/20608). @eugeneo would that be enough to add this support to the inspector? Do you need any specific data/in a specific format to be able to render HTTP requests?

eugeneo commented 6 years ago

Chrome has a well-define protocol for inspecting network - https://chromedevtools.github.io/devtools-protocol/tot/Network

It should not be too hard to write a handler that will handle the messages, the bigger problem is gathering the data. This may need an effort from some community packages, e.g. WebSocket packages will need to report events to Inspector, maybe something will need to be done to Express (though I expect HTTP support only needs instrumentation in the standard library)

mcollina commented 6 years ago

@eugeneo that seem very much focus on outgoing requests and not incoming, i.e. http clients vs http servers. How those will be displayed? Gathering the data would not be very hard with async_hooks or by plugging it into our internals, I can help with that.

eugeneo commented 6 years ago

Chrome protocol records both requests and responses.

mcollina commented 6 years ago

@eugeneo I don't think I've been clear enough, so let me ask with more details. In Node.js we have IncomingMessage and OutgoingMessage. The usage of those is swapped between the client and the server, so that a request on the client inherits from OutgoingMessage, while a request received by the server inherits from IncomingMessage (and vice versa for the responses). The inspector only have 2 types, Request and Response. How would you map the Node.js types to the Chrome protocol types?

If you have time, an example of sending that data from Node.js to the inspector would be brilliant, I'll see if I can do a prototype.

eugeneo commented 6 years ago

Not sure what difference would that be - Response always comes after the Request, on server or client.

There are likely some gotchas that can only be understood when the implementation is done. NodeTracing (and pending NodeWorker) domains are not 100% copies of the Chrome protocol, so NodeNetwork would be able to reflect specifics.

mcollina commented 6 years ago

I'm confident that I can get that data out, is it possible right now to send that data to the inspector?

eugeneo commented 6 years ago

No. Inspector will need a new "domain" added - e.g. https://github.com/nodejs/node/pull/21364/commits/dc1822d5ed2962b81ee776769f03087a3de50ff0

nicoburns commented 6 years ago

The inspector only have 2 types, Request and Response. How would you map the Node.js types to the Chrome protocol types?

Maybe always map the request to Request and the response to the Response (regardless of the exchange was initiated by the node process or by an external client, but allow filtering Incoming vs Outgoing requests?

mcollina commented 6 years ago

It should not be too hard to write a handler that will handle the messages, the bigger problem is gathering the data.

@eugeneo would you like to work together on this? I think we can solve the two pieces of the puzzle, as I think getting the data is feasible. I would recommend focusing on Request and Response from http first, and discuss websocket later.

eugeneo commented 6 years ago

@mcollina I can help with navigating inspector code and code reviews.

(UPD: slight edit)

mike-kaufman commented 5 years ago

@mcollina - is this the issue you wanted added to diag agenda?

mcollina commented 5 years ago

Yes thanks!

Il giorno gio 29 nov 2018 alle 00:51 Mike Kaufman notifications@github.com ha scritto:

@mcollina https://github.com/mcollina - is this the issue you wanted added to diag agenda?

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

eugeneo commented 5 years ago

I would like to participate in the discussion of this issue. Where can I find instructions on how to join the meeting?

mmarchini commented 5 years ago

Meetings of the working group typically occur every other Wednesday. A few days before each meeting, an issue will be created with the date and time of the meeting. The issue will provide schedule logistics as well as an agenda, links to meeting minutes, and information about how to join as a participant or a viewer.

https://github.com/nodejs/diagnostics#semi-monthly-meetings

mhdawson commented 5 years ago

@eugeneo meetings are shown on the nodejs.org/calendar and then an issue is posted a few days in advance which has the info for the meeting. See the issues referenced by this issue.

mike-kaufman commented 5 years ago

@mcollina - we're going to remove from diag agenda, since this needs a champion. Please add back if you disagree. Thanks!

cjihrig commented 5 years ago

I think this would be a great feature to have. Unfortunately, at this point in time I don't have the necessary expertise to implement this myself (probably the same boat @mcollina).

Would it be possible to get assistance or some heavy guidance from Google on how to proceed here? I've seen that we need to add a new domain, but that's probably still a bit too high for people that don't work on these tools regularly.

hashseed commented 5 years ago

Unfortunately I'm not all that familiar with how DevTools frontend works. What I do know though:

DevTools has a Node.js-specific UI that would need to be extended.
Node.js would need to implement the Network domain by implementing the existing end points of the protocol, as listed here, as a debugging agent.

mike-kaufman commented 5 years ago

One of the interesting exercises here is to determine amount of overlap between existing CrDP network protocol @hashseed cites above, and what is desirable to expose from node. This exercise, I think, would provide a basis for a more concrete discussion about what is desired to expose from node, whether a new CrDP domain is required or existing Network Domain can be re-used, and what UX changes are necessary.

jkrems commented 5 years ago

If we want to test properly, it may also be good to send something to Chrome (or DevTools at least) rather sooner than later if front-end changes are required to show the Network tab. While implementing the protocol from the spec is nice, I assume it would be a lot easier if it can be tested against the DevTools UI.

hashseed commented 5 years ago

I wouldn't be too worried about that. Chrome has a shorter release cycle than Node.js.

auchenberg commented 5 years ago

Hi everyone,

Just chiming in here. Initiator of remotedebug.org, and member of https://github.com/WICG/devtools-protocol

Most of the core Network related APIs has been stable since the Webkit days from which Chrome DevTools Protocol were forked from. You can use http://compatibility.remotedebug.org/Network to compare the API surface.

I wouldn't worry as long you implemented the non-experimental APIs, which should cover the basics for CDP compatible tools.

AndreasGassmann commented 5 years ago

I would really love to see this feature implemented. I'm currently writing tests for a library and just quickly wanted to check if I forgot to stub some of the API calls. I was actually a bit surprised to see that this isn't supported yet.

While we have to wait for this issue to be resolved, what are currently the best workarounds or tools available that solve this?

jkrems commented 5 years ago

Early notes:

The existing Network protocol descriptions that would need to be ported / migrated are here: https://github.com/ChromeDevTools/devtools-protocol/blob/master/pdl/browser_protocol.pdl
The browser Network domain depends on the Security domain but afaict only for its data types (e.g. getting certificate details for secure requests). The Security domain also allows to switch off certificate errors but I think we wouldn't need that in a first version.
The network domain includes a feature to intercept and modify requests/responses which would require a fairly deep integration if we'd want to support it.
Emulating network conditions may be interesting at some point (for code that doesn't just run in a data center/cloud somewhere) but feels like something we can punt on in the beginning.
The domain currently assumes a cookie jar which node doesn't really have. I assume we'd make those either noops or remove those functions from node's network domain.
The user agent override is most likely not really interesting but setting additional headers may be.
It looks like the current code generator doesn't handle external references to domains defined elsewhere (e.g. references to Runtime.StackTrace).

There are enough differences that a node-specific domain may be reasonable but I'd really like us to keep the original domain because I hope it would allow easier tooling integration. E.g. if VSCode adds support for inspecting network traffic, it can use the same code for doing it in Chrome/Edge and node.

quantuminformation commented 5 years ago

Would totally love this feature, I dread having to install wireshark in 2019.

gireeshpunathil commented 5 years ago

should this remain open? [ I am trying to chase dormant issues to closure ]

AndreasGassmann commented 5 years ago

I hope this does not get closed because it's still an issue.

Because this is currently not implemented, the easiest way for us to test if we mocked all of the requests in our unit tests is to take the computer that runs the tests offline and see which tests fail because of network errors.

Needless to say, I would rather have a network inspection tab in the devtools...

jkrems commented 5 years ago

One thing that has been happening is that @khanghoang has started implementing a node Network tab for ndb specifically: https://github.com/GoogleChromeLabs/ndb/pull/282

OliverJAsh commented 5 years ago

For anyone looking to do this today, you can use Charles proxy. Here's a guide: http://marianna.im/tech/capture-nodejs-traffic-with-charles/.

jkrems commented 5 years ago

Charles is definitely a great way to inspect network traffic in general and can be a super helpful workaround here!

But it only gets users half-way there. One of the powerful features of the Network tab is that it can tie the requests directly to the exact stack of what caused the request. And it would hopefully also add network info to the timeline, making it possible to combine it with custom marks. Still crossing my fingers that at least the ndb thing works out.

khanghoang commented 5 years ago

@jkrems Totally agree with you about the benefits of having the network tab inside the debugger. At PayPal, We use GraphQL to talk to a ton of services and we found the network tab is really helpful to debug the requests and responses.

I'm glad that you like my PR for ndb but unfortunately, I don't think it has the chance to get merged. I have been trying to ping the maintainer multiple times but no response.

naugtur commented 5 years ago

For those who are after the stack traces to the code making outgoing requests, check out the network tool from https://github.com/naugtur/debugging-aid
All you need to do to get it to work is node --require=debugging-aid/network app.js

nodejs / diagnostics

Support Network Inspection #75