Skip to content

src: do not enable the inspector during snapshot building - #67154

Open
joyeecheung wants to merge 1 commit into
nodejs:mainfrom
joyeecheung:fix-snapshot-inspector
Open

joyeecheung wants to merge 1 commit into
nodejs:mainfrom
joyeecheung:fix-snapshot-inspector

Conversation

@joyeecheung

@joyeecheung joyeecheung commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

V8 no longer allows attaching a debuger or setting a debug context ID during snapshot building. Avoid enabling the inspector in this case, and reject the inspector options when the user-land --build-snapshot is used.

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/8679045

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/inspector
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. config Issues and PRs related to Node.js configuration and feature settings. inspector Issues and PRs related to the V8 inspector protocol. needs-ci PRs that need a full CI run. labels Oct 31, 2026
@codecov

codecov Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.70%. Comparing base (45df6b7) to head (7feef72).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/inspector_agent.cc 50.00% 2 Missing and 1 partial ⚠️
src/node_options.cc 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #67154   +/-   ##
=======================================
  Coverage   90.69%   90.70%           
=======================================
  Files         797      797           
  Lines      278883   278898   +15     
  Branches    53630    53623    -7     
=======================================
+ Hits       252934   252961   +27     
+ Misses      16314    16301   -13     
- Partials     9635     9636    +1     
Files with missing lines Coverage Δ
src/node_options.h 95.83% <100.00%> (+0.12%) ⬆️
src/node_options.cc 88.21% <94.11%> (+0.02%) ⬆️
src/inspector_agent.cc 83.14% <50.00%> (-0.28%) ⬇️

... and 20 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.

@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.

LGTM but linter is complaining.

V8 no longer allows attaching a debuger or setting a debug context ID
during snapshot building. Avoid enabling the inspector in this case,
and reject the inspector options when the user-land --build-snapshot
is used.

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/8679045
Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>
@joyeecheung
joyeecheung force-pushed the fix-snapshot-inspector branch from b0271b9 to 7feef72 Compare November 2, 2026 17:14
@joyeecheung

Copy link
Copy Markdown
Member Author

Force-pushed to sign it off and fixed the linter complaint.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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++. config Issues and PRs related to Node.js configuration and feature settings. 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.

3 participants