Skip to content

worker: don't leak watch messages to listeners - #65049

Open
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:worker/watch-require-message-leak
Open

bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:worker/watch-require-message-leak

Conversation

@bitpshr

@bitpshr bitpshr commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Under node --watch, each worker reports its module dependencies to the watch supervisor over the worker's message channel. The main thread forwarded those watch:require/watch:import messages to the supervisor but then still emitted them to user worker.on('message') listeners, so applications received undocumented internal messages. It reproduces with node --watch on a script that spawns a Worker (even an empty one).

This consumes the forwarded watch dependency messages instead of surfacing them to user listeners. The main process's own dependency reports go through a separate process.send path in the loader, so watch tracking is unaffected.

The added regression test fails before the change (the watch:require messages reach the user listener) and passes after.

Fixes: #65044

Under `node --watch`, each worker reports its module dependencies to the
watch supervisor over the worker's message channel. The main thread
forwarded those `watch:require` and `watch:import` messages to the
supervisor but then still emitted them to user `worker.on('message')`
listeners, so applications received undocumented internal messages.

Consume the forwarded watch dependency messages instead of surfacing
them to user listeners.

Fixes: nodejs#65044
Signed-off-by: Paul Bouchon <mail@bitpshr.net>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. worker Issues and PRs related to the worker_threads module and Worker API. labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (4a5eb1c) to head (fbe4f64).
⚠️ Report is 168 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65049      +/-   ##
==========================================
+ Coverage   90.15%   90.28%   +0.13%     
==========================================
  Files         744      759      +15     
  Lines      242517   247637    +5120     
  Branches    45688    46693    +1005     
==========================================
+ Hits       218642   223585    +4943     
- Misses      15358    15509     +151     
- Partials     8517     8543      +26     
Files with missing lines Coverage Δ
lib/internal/worker.js 96.78% <100.00%> (+0.03%) ⬆️

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

MILLERMARRU

This comment was marked as low quality.

@bitpshr

bitpshr commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Hi @nodejs/workers, wanted to keep this on your radar. Small fix with a regression test, green since August. Happy to rebase if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. worker Issues and PRs related to the worker_threads module and Worker API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

watch mode triggers unsolicided messages to Workers

3 participants