Skip to content

inspector: fix connectToMainThread() abort without parent inspector - #65976

Open
codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:fix/embedder-connect-to-main-thread-abort
Open

inspector: fix connectToMainThread() abort without parent inspector#65976
codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:fix/embedder-connect-to-main-thread-abort

Conversation

@codebytere

Copy link
Copy Markdown
Member

inspector.Session#connectToMainThread() in a Worker aborts the process when the parent Environment was created with kNoCreateInspector, which is what an embedder that runs its own inspector (or none) passes: Agent::ConnectToMainThread() reaches CHECK_NOT_NULL(parent_handle_) because a parent without an inspector never hands the Worker a parent handle.

It now throws ERR_INSPECTOR_NOT_AVAILABLE in that case, next to the existing throw for a Worker whose own inspector is not initialized; the code is added to node_errors.h so C++ can raise the error lib already uses.

Tests: EnvironmentTest.WorkerConnectToMainThreadWithoutInspector starts a Worker under a kNoCreateInspector Environment and expects the error code; it aborted before.

Refs: #35025


Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/inspector

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol. needs-ci PRs that need a full CI run. labels Sep 11, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.00%. Comparing base (46bbfc4) to head (a058629).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65976      +/-   ##
==========================================
+ Coverage   89.98%   90.00%   +0.02%     
==========================================
  Files         784      784              
  Lines      268361   268364       +3     
  Branches    51117    51110       -7     
==========================================
+ Hits       241478   241539      +61     
+ Misses      17438    17381      -57     
+ Partials     9445     9444       -1     
Files with missing lines Coverage Δ
src/inspector_agent.cc 81.20% <100.00%> (+0.07%) ⬆️
src/node_errors.h 86.95% <ø> (ø)

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

`inspector.Session#connectToMainThread()` in a Worker aborted the
process on `CHECK_NOT_NULL(parent_handle_)` in
`Agent::ConnectToMainThread()` when the parent Environment was created
with `kNoCreateInspector`, as embedders that run their own inspector
(or none) do.

Throw the new `ERR_INSPECTOR_NOT_AVAILABLE` in that case, next to the
existing throw for a Worker whose own inspector is not initialized.

Refs: nodejs#35025
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
@codebytere
codebytere force-pushed the fix/embedder-connect-to-main-thread-abort branch from e6399c9 to a058629 Compare September 11, 2026 19:24
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

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