Skip to content
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: convert event params to protocol without json #57027

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

legendecas
Copy link
Member

Event params object can be converted to inspector protocol directly.
This also enables binary data, like Network.dataReceived, to be sent
in the inspector protocol from JavaScript since JSON representation
does not support plain binary data.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Feb 13, 2025
@legendecas legendecas added the inspector Issues and PRs related to the V8 inspector protocol label Feb 13, 2025
@legendecas legendecas force-pushed the inspector/stringify branch 2 times, most recently from 88567de to 2c4f439 Compare February 13, 2025 11:55
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 61.62162% with 71 lines in your changes missing coverage. Please review.

Project coverage is 89.07%. Comparing base (85f5a6c) to head (cdd0230).
Report is 81 commits behind head on main.

Files with missing lines Patch % Lines
src/inspector/network_agent.cc 54.83% 32 Missing and 38 partials ⚠️
src/inspector_js_api.cc 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #57027      +/-   ##
==========================================
- Coverage   89.11%   89.07%   -0.04%     
==========================================
  Files         665      666       +1     
  Lines      193193   193282      +89     
  Branches    37212    37256      +44     
==========================================
+ Hits       172158   172167       +9     
- Misses      13775    13815      +40     
- Partials     7260     7300      +40     
Files with missing lines Coverage Δ
lib/inspector.js 96.72% <100.00%> (+0.44%) ⬆️
src/inspector/network_inspector.cc 90.00% <100.00%> (ø)
src/inspector/network_inspector.h 100.00% <ø> (ø)
src/inspector/protocol_helper.h 100.00% <100.00%> (ø)
src/inspector_agent.cc 79.26% <100.00%> (+0.55%) ⬆️
src/inspector_agent.h 100.00% <ø> (ø)
src/inspector_js_api.cc 86.24% <83.33%> (-0.16%) ⬇️
src/inspector/network_agent.cc 61.65% <54.83%> (-33.54%) ⬇️

... and 25 files with indirect coverage changes

Event params object can be converted to inspector protocol directly.
This also enables binary data, like `Network.dataReceived`, to be sent
in the inspector protocol from JavaScript since JSON representation
does not support plain binary data.
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2025
@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member Author

@nodejs/inspector would you mind taking a look at this? Thank you!

@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member Author

@nodejs/inspector @nodejs/cpp-reviewers would you mind taking a look at this? Thank you!

@legendecas legendecas added the review wanted PRs that need reviews. label Feb 20, 2025
@legendecas legendecas added commit-queue Add this label to land a pull request using GitHub Actions. and removed review wanted PRs that need reviews. labels Feb 24, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2025
@nodejs-github-bot nodejs-github-bot merged commit b7beb33 into nodejs:main Feb 24, 2025
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in b7beb33

@legendecas legendecas deleted the inspector/stringify branch February 24, 2025 13:22
targos pushed a commit that referenced this pull request Feb 25, 2025
Event params object can be converted to inspector protocol directly.
This also enables binary data, like `Network.dataReceived`, to be sent
in the inspector protocol from JavaScript since JSON representation
does not support plain binary data.

PR-URL: #57027
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
targos pushed a commit that referenced this pull request Feb 25, 2025
Event params object can be converted to inspector protocol directly.
This also enables binary data, like `Network.dataReceived`, to be sent
in the inspector protocol from JavaScript since JSON representation
does not support plain binary data.

PR-URL: #57027
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants