Repository navigation
fix(worker): route worker_threads errors like the web surface, and fire once listeners once - #2065
Conversation
Sorry, something went wrong.
…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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info ⚙️ Run configuration
Reviewing files that changed from the base of the PR and between 42a8bcf and 1d9337c. 📒 Files selected for processing (3)
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 WalkthroughWorker 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. ChangesWorker event handling
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)
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. ❤️ ShareA rabbit taps a worker thread, Comment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
… 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.
|
✅ Action performed
Review finished.
|
Sorry, something went wrong.
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