-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
inspector: add initial support for network inspection #53593
inspector: add initial support for network inspection #53593
Conversation
Review requested:
|
499ab30
to
6a9a7cf
Compare
CC @nodejs/undici regarding support for |
Hey, this is incredibly cool. |
Yes, at the very least we should probably integrate with fetch/undici, it's hookable already through the dispatcher so it shouldn't be too hard (before this is stable, not to block this PR) |
I think the fetch api is also very important and I hope it can be supported |
@benjamingr I wasn't aware of undici's dispatcher feature, and I've now seen that undici includes diagnostics channels. I'm thinking using |
I received this update before taking a shower, and this piece can finally be pushed forward, which is great. About two weeks ago, I developed a library for this issue, and some key ideas should be useful: The 'v8 inspector' lacks a network domainIt is used How to listen for HTTP/HTTPS requests?Basic ideas
Key points
How to make full use of the
|
One thing I was prototyping years ago was implementing Inspector domains in the user land, in JS. I.e. instead of piping Another thing we discussed a lot in the past is that Network domain in Node should be reversed as most users will be interested in debugging server application. There should be Chrome DevTools at the time was very adamant not to reuse Chrome domains for the domains not in V8. Please rename this domain to |
Could we implement it in terms of diagnostic channnels that external libs (undici) can just hook into? |
I completely agree, and I also believe that the network domain in Node should be reversed. Now we can actually extend the v8 inspector launched by the node, but it is more related to remote debugging, and due to domain limitations, network and other domains cannot be implemented. |
Note needs both HTTP server and HTTP client support... It would be invaluable to be able to see request that was received and what was sent to other microservices. |
fc0ed45
to
5656be6
Compare
@nodejs/undici For network inspection on fetch API, we need undici's diagnostics_channel to support a hook when body is received. Has there been any progress on nodejs/undici#1342? |
I would like to confirm if listening to network requests through the undici library diagnostics.channel is compatible with previous libraries? |
@GrinZero I'm going to add as many features of the @eugeneo I need your advice on how to properly define and implement the custom |
5656be6
to
1c754b4
Compare
I am wondering if it is possible to be more open and allow the node v8 inspector to directly expose the websocket server. This way, developers can not only operate devtool as a CDP client, but also extend devtool as a CDP server in the future. |
1c754b4
to
6e727ee
Compare
dataReceived(request._inspectorRequestId, DateNow() / 1000, chunk.length); | ||
responseString += chunk.toString(); | ||
}; | ||
response.on('data', onData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the test results, consuming response data and entering flowing mode in the diagnostics_channel hook is causing some issues in the core. Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show the issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/nodejs/node/actions/runs/9696727280/job/26759268382?pr=53593
I'll investigate and summarize the issues later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this seems wrong - we can't consume the request this way :D
We'd need to 'tee' the stream (discussions happening elsewhere) though with the inspector cloning it should also be fine.
I think for this PR it would make sense to exclude request bodies and do it in a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped support for response body inspection for now. fbf5a31
@benjamingr Do you have any discussion links for reference?
We could add fetch support via nodejs/undici#2701. Generically I think we should add some APIs to let devs integrate 3rd party clients. |
+1 to this being diagnostics_channel based and +1 for letting userland tools integrate with it. |
6e727ee
to
b874c90
Compare
@danilsomsikov So, I would like to request to enable the display of the network tab on the frontend. What should I do to proceed? |
Please file a feature request at https://goo.gle/devtools-bug |
Requested at https://issues.chromium.org/issues/353924015. |
Landed in a523c34 |
PR-URL: #53593 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
I hacked a bit in DevTools frontend and documented the results here. Node.js implementation of the Network domain is incomplete still. |
Notable changes: deps: * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997 http: * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054 inspector: * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593 lib,src: * drop --experimental-network-imports (Rafael Gonzaga) #53822 meta: * add jake to collaborators (jakecastelli) #54004 module: * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725 * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619 stream: * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111 test_runner: * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866 * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853 * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853 PR-URL: TODO
Notable changes: deps: * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997 http: * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054 inspector: * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593 lib,src: * drop --experimental-network-imports (Rafael Gonzaga) #53822 meta: * add jake to collaborators (jakecastelli) #54004 module: * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725 * (SEMVER-MINOR) unflag detect-module (Geoffrey Booth) #53619 stream: * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111 test_runner: * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866 * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853 * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853 PR-URL: #54123
Notable changes: deps: * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997 http: * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054 inspector: * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593 lib,src: * drop --experimental-network-imports (Rafael Gonzaga) #53822 meta: * add jake to collaborators (jakecastelli) #54004 module: * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725 stream: * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111 test_runner: * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866 * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853 * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853 PR-URL: #54123
Notable changes: deps: * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997 http: * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054 inspector: * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593 lib,src: * drop --experimental-network-imports (Rafael Gonzaga) #53822 meta: * add jake to collaborators (jakecastelli) #54004 module: * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725 stream: * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111 test_runner: * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866 * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853 * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853 PR-URL: #54123
Notable changes: deps: * (SEMVER-MINOR) V8: backport 7857eb34db42 (Stephen Belanger) #53997 http: * (SEMVER-MINOR) add diagnostics channel `http.client.request.error` (Kohei Ueno) #54054 inspector: * (SEMVER-MINOR) add initial support for network inspection (Kohei Ueno) #53593 lib,src: * drop --experimental-network-imports (Rafael Gonzaga) #53822 meta: * add jake to collaborators (jakecastelli) #54004 module: * (SEMVER-MINOR) add --experimental-strip-types (Marco Ippolito) #53725 stream: * (SEMVER-MINOR) expose DuplexPair API (Austin Wright) #34111 test_runner: * (SEMVER-MINOR) fix support watch with run(), add globPatterns option (Matteo Collina) #53866 * (SEMVER-MINOR) refactor snapshots to get file from context (Colin Ihrig) #53853 * (SEMVER-MINOR) add context.filePath (Colin Ihrig) #53853 PR-URL: #54123
Saw this on the Node 22.6.0 release notes, super exciting! I'm going to get inspection support prioritized for VS Code's JS debugger soon 🙂 |
As I mentioned before, the network requests received through the So is it possible for us to consider other ways to achieve network monitoring? For example, directly modifying the source code of HTTP and HTTPS, or hijacking the request methods of HTTP and HTTPS under specific conditions. However, I'm sorry, but for now, I can only provide assistance in terms of ideas on how to implement it. Unless I can learn how to debug the Here is also a brief introduction to my open source project. Before node fully supports network debugging, this library can be used. Among them, the monitoring of network requests is achieved through hijacking HTTP, HTTPS, and fetch libraries, which currently runs well and supports most devtool functions. |
I've implemented some inspection in VS Code, but probably won't ship it in the current version since pretty much the only info we get (in stable 22.6.0) is that "a request to this URL happened." Once some support for information like status codes and bodies come in I'll be eager to flip the switch! not as pretty as chrome devtools, but only a couple hours of work ;) |
@connor4312 Thank you for your quick work! This is an awesome feature 🤩. In the next release, we'll be adding additional information such as headers and status codes. You can track our progress through this issue: #53946 :) |
A text for the release notes
The idea of supporting network inspection in Node.js was first proposed 7 years age in the nodejs/diagnostics#75. Despite numerous discussions, we have yet to settle on an implementation approach. This PR aims to serve as a starting point to explore and refine how we can achieve this feature. This PR introduces basic support for the Network domain of the Chrome DevTools Protocol (CDP) and its corresponding agent implementation in Node.js. Although this is an initial implementation with several pending tasks, it sets a foundation to verify if we are heading in the right direction.
Summary
This description outlines the strategy to support network inspection in Node.js and the design of the APIs that allow third-party libraries to integrate with the network inspection. Specifically, it introduces the
NodeNetwork
domain, a Node.js-specific extension of the standardNetwork
domain, which supports both client and server application network activities.User stories
As a client app developer
I want to be able to check the network activities triggered by the client APIs (the
http
module,fetch
API, andWebSocket
API) on devtools such as Chrome DevTools when I run the app in debugging mode vianode --inspect index.js
.As a serer app developer
I want to be able to check the network traffics happened in my server on devtools such as Chrome DevTools when I run the app in debugging mode via
node --inspect index.js
.As a HTTP client library developer
I want to enable my library to integrate with network inspection. For example, when my library sends a HTTP request and receives a HTTP response, it sends protocol events with debugging data to allow a library user to inspect them on devtools.
Design
Tracking network activities
Network activities can be captured within
diagnostics_channel
hooks. This approach enables us to monitor activities in both core modules (http
,https
) and external libraries (undici
) without changing the core implementation.Emit protocol events to DevTools
Network activities captured in
diagnostics_channel
are passed to the inspector agent using theinspector.NodeNetwork
API as customNodeNetwork
domain objects. The customNodeNetwork
domain extends the standardNetwork
domain by including Node.js-specific events and commands, enabling more granular and relevant tracking of network activities specific to the Node.js environment. It also allows third-party libraries to integrate with Node.js's inspector mechanism.The
NodeNetwork
domain will support both client activities (such as a request sent from the client) and server activities (such as a request received by the server).When
NodeNetwork
event is sent, Node.js internally sends some of theNetwork
domain events to the devtools frontend so that Chrome DevTools can capture them and show network activities in the network panel.Demo
Currenlty, the Node-specific DevTools Frontend lacks a network tab. Therefore, you'll need to use the Chrome DevTools Frontend, accessible via
devtools://devtools/bundled/inspector.html
. Below is a simple demonstration:Network activity sources
These APIs can be supported once each
diagnostics_channel
provides sufficient hook timing and resources.http
modulehttps
modulehttp2
moduleScope of this PR
This PR aims to provide a minimal implementation for network inspection, focusing on delivering the fundamental functionalities. The tasks accomplished in this PR include:
diagnositcs_channel
, targetinghttp
andhttps
GET requests.NodeNetwork
domain.Network
domain.Future work
To fully support the Network domain of the CDP, several tasks remain:
Network.loadingFailed
Limitations and Challenges
cc @nodejs/inspector @eugeneo