Skip to content

inspector: capture HTTP body before decoding - #64496

Closed
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:inspector
Closed

inspector: capture HTTP body before decoding#64496
Archkon wants to merge 1 commit into
nodejs:mainfrom
Archkon:inspector

Conversation

@Archkon

@Archkon Archkon commented Jul 14, 2026

Copy link
Copy Markdown

Fixes: #64494

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/inspector
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. labels Jul 14, 2026
@Archkon

This comment was marked as spam.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
βœ… Project coverage is 90.31%. Comparing base (c32ea43) to head (3411583).
⚠️ Report is 54 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/inspector/network_http.js 80.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64496      +/-   ##
==========================================
- Coverage   90.32%   90.31%   -0.02%     
==========================================
  Files         759      760       +1     
  Lines      248326   248537     +211     
  Branches    46859    46903      +44     
==========================================
+ Hits       224293   224458     +165     
- Misses      15481    15506      +25     
- Partials     8552     8573      +21     
Files with missing lines Coverage Ξ”
lib/_http_common.js 99.43% <100.00%> (+0.01%) ⬆️
lib/internal/inspector/network_http.js 93.97% <80.00%> (-1.73%) ⬇️

... and 79 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Archkon

This comment was marked as spam.

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of an ad-hoc symbol method for this. What about adding a diagnostic channel dedicated for http body chunks?

Also I'd like to hear from @nodejs/http about any suggestions on this?

@Archkon

This comment was marked as spam.

igochkov added a commit to igochkov/vscode-regelspraak that referenced this pull request Aug 9, 2026
* Run CI on Node 24 and the current action majors

Same move as the server repository, for the same reasons: the runner was
forcing actions/checkout@v4 and actions/setup-node@v4 onto Node 24 and
warning about it, and Node 20 went end-of-life in April 2026 while the
extension host this client runs in is on 24.

* Turn off the debugger's network view for F5

Opening any .rgs file in the Extension Development Host logged a
TypeError from deep inside Node, with no frame belonging to this
extension:

  TypeError: Missing dataLength in event
      at broadcastToFrontend (node:inspector:212:3)
      at Object.dataReceived (node:inspector:221:29)
      at IncomingMessage.<anonymous> (node:internal/inspector/network_http:140:13)

It is nodejs/node#64494. VS Code starts every extension host with
--experimental-network-inspection, and js-debug enables the Network
domain for the session because debug.javascript.enableNetworkView
defaults to true. Node's instrumentation then reads chunk.byteLength on
each response chunk β€” which is undefined once anything calls
res.setEncoding(), because string chunks have length, not byteLength β€”
and the runtime rejects the event.

The throw is not cosmetic: it runs in a listener registered ahead of the
consumer's own, so the offending request's body is swallowed. Nothing
here makes HTTP requests (the client talks to the server over IPC), so
this only ever hits some other extension loaded into the development
host, but it is noise on every debug session.

A fix is pending upstream in nodejs/node#64496; drop this setting once
the runtime carries it.

* Type against Node 24 as well as build on it

Matches the server repository: bumping only the runtime left the type
surface a major behind the extension host we ship into.

@types/node 24 pulls in esnext.disposable, which puts [Symbol.dispose]
on MapIterator. vscode-jsonrpc's LinkedMap β€” reached here through
vscode-languageclient β€” extends Map but declares keys()/values()/
entries() as IterableIterator, so the library's own .d.ts no longer
type-checks (TS2416). vscode-jsonrpc 9.0.1, the newest, declares it the
same way, so upgrading is not a way out; skipLibCheck is.

A forced full rebuild and lint are both clean.
@Archkon

This comment was marked as spam.

Capture HTTP response body chunks before stream decoding so network
inspection receives the original bytes when user code calls
response.setEncoding().

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
@aduh95 aduh95 closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

inspector: response.setEncoding() causes response.on('data') never triggered when --experimental-network-inspection is enabled

4 participants