-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
inspector: support inspecting HTTP/2 request and response bodies #60483
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
base: main
Are you sure you want to change the base?
inspector: support inspecting HTTP/2 request and response bodies #60483
Conversation
Signed-off-by: Darshan Sen <raisinten@gmail.com>
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60483 +/- ##
========================================
Coverage 88.56% 88.57%
========================================
Files 704 704
Lines 207774 207955 +181
Branches 40027 40065 +38
========================================
+ Hits 184020 184199 +179
+ Misses 15800 15786 -14
- Partials 7954 7970 +16
🚀 New features to boost your workflow:
|
| }, | ||
| }); | ||
|
|
||
| stream.on('data', (chunk) => { |
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.
Attaching a data listener puts the stream into flowing mode. This is a significant side-effect and could break user code if they didn't attach the data listener immediately.
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.
@legendecas good catch, what do you think about using the readable stream channel I pushed in the last commit?
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.
I have opened a separate PR for this #60514
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.
Just use EE.prototype.on.call(stream, 'data', ...)
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.
Okay, pushed the change.
This would mean that network inspection of HTTP/2 calls would be slightly different from that of fetch, as it won't show the response body unless the user code actually reads it, whereas fetch calls display it even if the user code doesn't read it.
…le stream Signed-off-by: Darshan Sen <raisinten@gmail.com>
…a readable stream" This reverts commit a6c2fce.
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Now, we can track the request and response bodies of HTTP/2 calls through the Network tab of Chrome DevTools for Node.js.
Depends on: #60480
Refs: #53946
Demo
demo.mov