inspector: support for worker inspection in chrome devtools #56759

islandryu · 2025-01-25T13:31:05Z

This PR introduces support for inspecting workers in Chrome DevTools. I would like to gather some feedback to confirm whether there seem to be no issues with this approach. If the approach in this PR deviates significantly from the desired direction, I am happy to close it. However, if the general approach seems acceptable, I will proceed with this PR and submit a change request to Chrome DevTools.

Summary of Changes

This implementation uses the attachedToTarget event to notify the creation of workers. More details on the protocol can be found [here](https://chromedevtools.github.io/devtools-protocol/tot/Target/#event-attachedToTarget).

This sequence is how to worker inspection works.
When a worker is created, attachedToTarget is notified to the server, and the server thread allocates methods from the client to the appropriate thread based on the sessionId.

sequenceDiagram
    participant DevtoolClient as DevTool Client
    participant ServerThread as Server Thread
    participant MainThread as Main Thread
    participant WorkerThread as Worker Thread

    DevtoolClient ->> ServerThread: attach
    ServerThread ->> MainThread: attach

    MainThread ->> MainThread: execute program

    MainThread ->> WorkerThread: create worker

    MainThread ->> ServerThread: attachedToTarget
    ServerThread ->> DevtoolClient: attachedToTarget

    DevtoolClient ->> ServerThread: debugger method
    ServerThread ->> ServerThread: Check which thread to send to by sessionId
    ServerThread ->> WorkerThread: Send to main thread or worker thread

How to Verify

Currently, dedicated DevTools for Node.js do not support worker targets. For verification, use inspector.html:

devtools://devtools/bundled/inspector.html?ws=127.0.0.1:9229/(sessionId)

Steps:

Start the Node.js process with the following command:

node --inspect-brk --experimental-worker-inspection index.js

Use the following index.js and worker.js files for testing:

index.js:

const { Worker } = require('worker_threads');
const workerPath =  __dirname + '/worker.js';
const worker = new Worker(workerPath);

worker.js:

console.log("worker thread");
process.on('exit', () => {
  console.log('Worker1: Exiting...');
});

~~Currently, the results of console.log within the worker are being sent twice, but I'm working on fixing this issue.~~

Additional Information

The approach used in this PR follows the CDP (Chrome DevTools Protocol) and is different from the Node.js worker debugging protocol used in VSCode. It adheres to the existing CDP protocol to ensure compatibility.

fix #56343

nodejs-github-bot · 2025-01-25T13:31:11Z

Review requested:

@nodejs/gyp
@nodejs/inspector

lpinca · 2025-01-25T19:49:23Z

test/parallel/test-inspector-worker-target.js

+  }));
+}
+
+test().then(common.mustCall());


I think the test() function wrapper is no longer needed: by default, an unhandled promise rejection makes the process exit with a non-zero exit code.

Thank you.
I removed the wrapper.

legendecas · 2025-01-26T15:04:06Z

Tracing the history discussion, it would be great to have @eugeneo, @pavelfeldman and @dgozman weighing in this work

legendecas · 2025-01-29T13:52:02Z

Confirmed with Chrome DevTools team (dsv@, caseq@) that they are good with Node.js implementing the Target domain for worker discovery, or implementing NodeWorker domain support in Chrome DevTools frontend.

As the Target domain in this PR is behind the flag --experimental-worker-inspection, I believe it is fine to go ahead. Thank you for working on this!

islandryu · 2025-01-29T16:46:02Z

Confirmed with Chrome DevTools team (dsv@, caseq@) that they are good with Node.js implementing the Target domain for worker discovery, or implementing NodeWorker domain support in Chrome DevTools frontend.

As the Target domain in this PR is behind the flag --experimental-worker-inspection, I believe it is fine to go ahead. Thank you for working on this!

Thank you!
I'll resolve the conflicts, make a few minor fixes, and then open the PR.

Fixes: nodejs#56343

codecov · 2025-01-30T15:09:35Z

Codecov Report

Attention: Patch coverage is 94.73684% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.16%. Comparing base (0edeafd) to head (b669db4).
Report is 108 commits behind head on main.

Files with missing lines	Patch %	Lines
src/inspector/target_agent.cc	95.83%	0 Missing and 3 partials ⚠️
src/inspector_io.cc	93.18%	0 Missing and 3 partials ⚠️
src/inspector_agent.cc	91.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56759      +/-   ##
==========================================
- Coverage   89.21%   89.16%   -0.05%     
==========================================
  Files         663      667       +4     
  Lines      192001   193305    +1304     
  Branches    36923    37204     +281     
==========================================
+ Hits       171290   172358    +1068     
- Misses      13582    13697     +115     
- Partials     7129     7250     +121

Files with missing lines	Coverage Δ
src/inspector/main_thread_interface.h	`95.00% <100.00%> (+1.66%)`	⬆️
src/inspector/target_agent.h	`100.00% <100.00%> (ø)`
src/node_options.cc	`87.97% <100.00%> (+0.01%)`	⬆️
src/node_options.h	`98.34% <100.00%> (+<0.01%)`	⬆️
src/inspector_agent.cc	`79.83% <91.66%> (+0.08%)`	⬆️
src/inspector/target_agent.cc	`95.83% <95.83%> (ø)`
src/inspector_io.cc	`92.56% <93.18%> (-0.02%)`	⬇️

... and 93 files with indirect coverage changes

islandryu · 2025-02-03T13:11:42Z

Issue to devtools-frontend
https://issues.chromium.org/issues/393746702

legendecas · 2025-02-04T10:57:06Z

test/parallel/test-inspector-worker-target.js

+  const sessionId = '1';
+  await session.waitForNotification('Target.targetCreated');
+  await session.waitForNotification((notification) => {
+    return notification.method === 'Target.attachedToTarget' &&


Event Target.attachedToTarget should not be emitted without a Target.setAutoAttach command or Target.attachToTarget command.

islandryu · 2025-02-08T11:19:26Z

src/inspector/target_agent.cc

+// TODO(islandryu): Currently, setAutoAttach applies the main thread's value to
+// all threads. Modify it to be managed per worker thread.
+crdtp::DispatchResponse TargetAgent::setAutoAttach(
+    bool auto_attach, bool wait_for_debugger_on_start) {
+  auto_attach_ = auto_attach;
+  wait_for_debugger_on_start_ = wait_for_debugger_on_start;
+
+  if (auto_attach) {
+    for (auto& target : targets_) {
+      if (!target.attached) {
+        target.attached = true;
+        attachedToTarget(target.worker,
+                         target.target_id,
+                         target.type,
+                         target.title,
+                         target.url);
+      }
+    }
+  }


setAutoAttach is implemented.
However, I am not able to determine autoAttach for each related Target and waitForDebuggerOnStart.
I have tried several way, but it was difficult to make the changes without affecting the existing nodeWorker, so I would like to make a TODO.

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 Jan 25, 2025

lpinca reviewed Jan 25, 2025

View reviewed changes

legendecas added the inspector Issues and PRs related to the V8 inspector protocol label Jan 29, 2025

legendecas linked an issue Jan 29, 2025 that may be closed by this pull request

Debug worker_threads #26609

Open

islandryu added 2 commits January 30, 2025 15:44

inspector: support for worker inspection in chrome devtools

525a2d2

Fixes: nodejs#56343

fix/test

cab3f96

islandryu force-pushed the feat/cdpTarget branch from 141ec5a to 984f640 Compare January 30, 2025 10:04

format

fafadd0

islandryu force-pushed the feat/cdpTarget branch from 984f640 to fafadd0 Compare January 30, 2025 10:34

islandryu changed the title ~~[WIP] inspector: support for worker inspection in chrome devtools~~ inspector: support for worker inspection in chrome devtools Jan 30, 2025

islandryu marked this pull request as ready for review January 30, 2025 11:07

islandryu marked this pull request as draft January 30, 2025 12:06

format

234a139

islandryu marked this pull request as ready for review January 30, 2025 12:15

fix test

01011f6

legendecas reviewed Feb 4, 2025

View reviewed changes

support setAutoAttach

cde7b09

islandryu commented Feb 8, 2025

View reviewed changes

fix stoi error

b669db4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inspector: support for worker inspection in chrome devtools #56759

inspector: support for worker inspection in chrome devtools #56759

islandryu commented Jan 25, 2025 •

edited

Loading

nodejs-github-bot commented Jan 25, 2025 •

edited by legendecas

Loading

lpinca Jan 25, 2025

islandryu Jan 27, 2025

legendecas commented Jan 26, 2025

legendecas commented Jan 29, 2025

islandryu commented Jan 29, 2025

codecov bot commented Jan 30, 2025 •

edited

Loading

islandryu commented Feb 3, 2025

legendecas Feb 4, 2025

islandryu Feb 8, 2025

inspector: support for worker inspection in chrome devtools #56759

Are you sure you want to change the base?

inspector: support for worker inspection in chrome devtools #56759

Conversation

islandryu commented Jan 25, 2025 • edited Loading

Summary of Changes

How to Verify

Steps:

Additional Information

nodejs-github-bot commented Jan 25, 2025 • edited by legendecas Loading

lpinca Jan 25, 2025

Choose a reason for hiding this comment

islandryu Jan 27, 2025

Choose a reason for hiding this comment

legendecas commented Jan 26, 2025

legendecas commented Jan 29, 2025

islandryu commented Jan 29, 2025

codecov bot commented Jan 30, 2025 • edited Loading

Codecov Report

islandryu commented Feb 3, 2025

legendecas Feb 4, 2025

Choose a reason for hiding this comment

islandryu Feb 8, 2025

Choose a reason for hiding this comment

islandryu commented Jan 25, 2025 •

edited

Loading

nodejs-github-bot commented Jan 25, 2025 •

edited by legendecas

Loading

codecov bot commented Jan 30, 2025 •

edited

Loading