Skip to content

fix(worker): route worker_threads errors like the web surface, and fire once listeners once - #2065

Open
adrian-niculescu wants to merge 2 commits into
NativeScript:feat/worker-threadsfrom
adrian-niculescu:fix/worker-threads-error-routing
Open

adrian-niculescu wants to merge 2 commits into
NativeScript:feat/worker-threadsfrom
adrian-niculescu:fix/worker-threads-error-routing

Conversation

@adrian-niculescu

@adrian-niculescu adrian-niculescu commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

A parentPort.on("message") listener that throws never reaches the worker's onerror or the parent's worker.on("error"). The relay dispatched without rethrowing, so the throw went to the uncaught-error reporter and stopped there. It now dispatches the way worker-global message delivery does.

An error that a worker.on("error") listener handled was also reported to the parent's global scope as unhandled, because the Worker's onerror returned nothing. It now returns whether a listener ran, which cancels the event, and the emitter reports that the way Node's emit does.

A once listener could fire twice: when an earlier listener emitted the same event again, the nested emit fired and removed it, and the outer emit then called it a second time from its snapshot. A once registration now records that it fired, as Node's once wrapper does, so a once listener an earlier listener removed still fires, as in Node.

Stacked on #2043; the same fix for iOS is NativeScript/ios#491. All three new specs fail on that branch and pass here, and the full device suite passes.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed worker error handling so errors are correctly treated as handled when an error listener is registered, rather than also triggering a global error.
    • Fixed one-time event listeners so they run only once, even when the same event is emitted again during a listener callback.
    • Errors thrown by worker message handlers now reach the parent worker’s error listener.

…errors like the web surface

A parentPort listener that threw was dispatched without rethrowing, so the error went to the uncaught-error reporter and never reached the worker's onerror or its parent. The relay now dispatches the way worker-global message delivery does.

The worker_threads Worker's onerror handler returned nothing, so an error its 'error' listeners took was also reported to the parent's global scope as unhandled. It returns whether a listener ran, which cancels the event.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fcdb1759-cd45-4845-b78b-1e041e866eb2
📥 Commits

Reviewing files that changed from the base of the PR and between 42a8bcf and 1d9337c.

📒 Files selected for processing (3)
  • test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js
  • test-app/app/src/main/assets/app/tests/testMessaging.js
  • test-app/runtime/src/main/cpp/js/node-worker-threads.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Worker event handling now returns listener-presence status, prevents recursive emission from invoking a once() listener more than once, and routes exceptions from worker-scope message listeners to the worker’s "error" event. Regression specs cover these behaviors.

Changes

Worker event handling

Layer / File(s) Summary
Emitter and once-listener semantics
test-app/runtime/src/main/cpp/js/node-worker-threads.js, test-app/app/src/main/assets/app/tests/testMessaging.js
WorkerEmitter.emit returns false when no listeners exist and true after dispatch. A once() registration is marked fired before invocation. A regression spec checks recursive emission.
Worker error relay
test-app/runtime/src/main/cpp/js/node-worker-threads.js, test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js, test-app/app/src/main/assets/app/tests/testMessaging.js
The worker error handler returns the result of emitting "error". Worker-scope message dispatch uses dispatchEventRethrowing. Regression specs check error-listener delivery, global error suppression, and delivery of a thrown parentPort listener error.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 1d933

The changed worker error and once-listener paths appear ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the worker error-routing fix and the once-listener behavior change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit taps a worker thread,
Then listens for the words it said.
A once-listener hops just once,
An error finds its listener, too.
The rabbit nods and bounds away.

Comment @coderabbitai help to get the list of available commands.

… reentrant emit

An earlier listener that emitted the same event again let the nested emit fire and remove a later once listener, and the outer emit then called it a second time from its snapshot. A once registration now records that it fired, as Node's once wrapper does.
@adrian-niculescu adrian-niculescu changed the title fix(worker): route parentPort listener throws and handled Node-style errors like the web surface fix(worker): route worker_threads errors like the web surface, and fire once listeners once Oct 6, 2026
@adrian-niculescu
adrian-niculescu marked this pull request as ready for review October 6, 2026 19:19
@adrian-niculescu

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant