inspector,http: support builtin http request bodies#62915
inspector,http: support builtin http request bodies#62915GrinZero wants to merge 14 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
metcoder95
left a comment
There was a problem hiding this comment.
lgtm, it just seems that is gonna need a documentation for the new dcs
|
Thanks for the feedback. I’ve updated the documentation in Specifically, I added/updated entries for:
I also aligned the wording and structure with the existing Node.js docs style for built-in channels. |
|
A slight push, hoping that a maintainer will notice. |
|
Sorry for the ping, @legendecas If you have some time, could you please help review this PR and trigger CI? This PR fixes an issue where the network inspector does not show Thanks a lot! |
|
push, request-ci plz |
|
Ill try to review the PR later, I think it's important we make sure it doesn't leak memory or keep the body buffer reference alive, which can exhaust resource quickly @ShogunPanda ptal |
I'll go explore the issue you mentioned |
|
Thanks for raising this concern. I did a focused memory/lifetime check for this PR, mainly around two questions:
Based on the current implementation and local testing, I do not see evidence that this PR keeps the original JS buffers alive. For both request and response bodies, the payload bytes are copied into inspector-owned storage before being retained by So the inspector is retaining copied payload bytes, not the original userland Buffer objects. I also ran a repeated-request memory test with explicit |
|
The test plan above was proposed by AI, and I reviewed it. For the first test, it uses The second test is a stress test. It lowers |
ab906d5 to
194a5fd
Compare
|
I merged the main branch code to avoid CI errors. |
194a5fd to
ba8f093
Compare
Thanks 🙏 I think we should add the request-ci tag to trigger the next steps. I have synced the code of the main branch |
aace601 to
16da9f4
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62915 +/- ##
==========================================
+ Coverage 89.65% 89.68% +0.03%
==========================================
Files 712 712
Lines 220512 221491 +979
Branches 42289 42453 +164
==========================================
+ Hits 197690 198637 +947
- Misses 14663 14692 +29
- Partials 8159 8162 +3
🚀 New features to boost your workflow:
|

Summary
This PR adds builtin
http/httpsrequest-body support to network inspection, soNetwork.getRequestPostDatacan return text request bodies while preserving the existing rejection behavior for binary request bodies.It also moves builtin
httpresponse-body tracking to a raw-byte hook beforeIncomingMessagedecoding, so response inspection remains correct even when user code callsresponse.setEncoding(...).In addition, this PR extends
requestWillBeSentto accept initiator data from JS diagnostics payloads and validates structuredinitiator.stackobjects against the inspector protocol schema before forwarding them to DevTools.Problem
Builtin
http/httpsnetwork inspection currently emits:Network.requestWillBeSentNetwork.responseReceivedNetwork.loadingFinishedHowever, the builtin
httpclient path does not currently expose request-body data to the inspector. As a result,Network.getRequestPostDatacannot return POST data for builtinhttp/httpsrequests.There are two related gaps as well:
IncomingMessage'data'events are not a stable source of raw bytes. If user code callsresponse.setEncoding('utf8'), the chunks observed by userland become strings, while the inspector protocol expects byte-oriented payloads.requestWillBeSentneeds to accept JS-provided initiator data, but structured stack trace input must be validated before it is forwarded as protocol data.Approach
1. Reuse the existing request buffering pipeline
This change does not alter the CDP schema or the existing buffering behavior in
NetworkAgent.Instead, it reuses the current pipeline already used by other transports:
2. Add builtin
httprequest-body diagnostics eventsThe builtin
httpclient now publishes:http.client.request.bodyChunkSenthttp.client.request.bodySentThese events are emitted from the
ClientRequestwrite path before HTTP framing is applied, so the inspector sees the original user payload rather than chunked-transfer framing bytes.3. Capture builtin
httpresponse bytes before decodingFor responses, this PR avoids relying on
IncomingMessage.on('data')innetwork_http.js.Instead, it adds:
http.client.response.bodyChunkReceivedThis hook runs before
Readable.setEncoding()transforms chunks for userland, so the inspector always receives raw bytes.4. Support and validate network initiator payloads
Network.requestWillBeSentcan now use JS diagnostics payloads to provide initiator data.On the C++ side, the incoming JS object is converted into inspector protocol values, and
initiator.stackis validated against theRuntime.StackTraceschema before being attached to the event. If the initiator payload is malformed, the event is rejected with a clear error instead of forwarding bad protocol data to the frontend.If JS does not provide an initiator, the existing behavior remains unchanged: the inspector captures the current stack trace and emits a default
scriptinitiator.5. Document the new diagnostics channels
This PR also updates
diagnostics_channeldocs to cover the new builtin HTTP client request/response body channels and their message shapes.Behavior
After this change:
httpandhttpsPOST requests with UTF-8 text bodies are available throughNetwork.getRequestPostDatahttpresponse inspection continues to work even if user code callsresponse.setEncoding('utf8')Network.requestWillBeSentcan carry JS-provided initiator stack dataTests
This PR adds and extends coverage in:
test/parallel/test-diagnostics-channel-http.jstest/parallel/test-inspector-network-http.jstest/parallel/test-inspector-network-arbitrary-data.jstest/parallel/test-inspector-emit-protocol-event-errors.jsThe updated tests cover:
write()andend()httpandhttpsNetwork.getRequestPostDataresponse.setEncoding('utf8')Verification
Verified locally with:
Refs