Skip to content

worker: fix messageerror listener cleanup - #65783

Closed
kiliczsh wants to merge 1 commit into
nodejs:mainfrom
kiliczsh:fix-worker-messageerror-listener-cleanup
Closed

worker: fix messageerror listener cleanup#65783
kiliczsh wants to merge 1 commit into
nodejs:mainfrom
kiliczsh:fix-worker-messageerror-listener-cleanup

Conversation

@kiliczsh

@kiliczsh kiliczsh commented Sep 4, 2026

Copy link
Copy Markdown

Worker exit cleanup attempts to remove messageerror listeners before
emitting the exit event, but uses the non-existent messageerrors event
name. As a result, messageerror listeners are still present while exit
listeners run, unlike message listeners.

Use the correct event name during cleanup. Add a regression test covering
both normal worker exit and termination with worker.terminate().

Fixes: #65782

Remove messageerror listeners before emitting the worker exit event.

Signed-off-by: Muhammed KΔ±lΔ±Γ§ <muhammeddkilicc@gmail.com>
@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 Sep 4, 2026
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.06%. Comparing base (7551e1d) to head (c21a0a2).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65783   +/-   ##
=======================================
  Coverage   90.05%   90.06%           
=======================================
  Files         769      769           
  Lines      261396   261396           
  Branches    49631    49620   -11     
=======================================
+ Hits       235399   235414   +15     
+ Misses      17034    17020   -14     
+ Partials     8963     8962    -1     
Files with missing lines Coverage Ξ”
lib/internal/worker.js 97.02% <100.00%> (ΓΈ)

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

@MikeMcC399

Copy link
Copy Markdown
Contributor

The recommendations in:

advise you should tackle only one issue at a time and that you should not open any new PRs until your first PR has been approved.

You already have #65776 open and this has not yet been reviewed or approved.

@kiliczsh

kiliczsh commented Sep 6, 2026

Copy link
Copy Markdown
Author

Thanks for heads up @MikeMcC399. I will close this PR and watch #65776.

@kiliczsh kiliczsh closed this Sep 6, 2026
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.

worker: messageerror listeners remain during exit

3 participants