Repository navigation
fix(worker): route worker_threads errors like the web surface, and fire once listeners once - #2065
Conversation
…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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 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. 📝 WalkthroughWalkthroughWorker event handling now returns listener-presence status, prevents recursive emission from invoking a ChangesWorker event handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The changed worker error and once-listener paths appear ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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. A rabbit taps a worker thread, Comment |
… 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
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
…rom the thrown value A worker_threads 'error' listener received the runtime's ErrorEvent, not an Error, and the message it carried was V8's "Uncaught Error: ..." text. The worker now reads the thrown value's name and message and forwards them with the error payload. The parent rebuilds an Error from them with the worker's stack, using the built-in constructor the name belongs to, and the parent's global error event carries that same Error when nothing handled it. The Worker shim cancels an error its listeners took with preventDefault() rather than through the truthy-return contract of onerror.
…r throws Reading the stack of the error a worker's onerror threw could run a `stack` getter, and a getter that threw replaced that error in the TryCatch holding it, so the parent rebuilt the getter's error instead. The stack read now runs under its own TryCatch. The thrown value's name and message travel to the parent as UTF-16, so an unpaired surrogate arrives as thrown rather than as U+FFFD. ArgConverter::ConvertToString copies by length, so an embedded NUL no longer cuts a converted string short.
This branch has not been deployed
1 participant
A
parentPort.on("message")listener that throws never reaches the worker'sonerroror the parent'sworker.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.A
worker.on("error")listener received the runtime'sErrorEventinstead of anError, and an error it handled was also reported to the parent's global scope as unhandled. The worker now sends the thrown value'snameandmessagewith the error payload, and the parent rebuilds anErrorfrom them and the worker's stack, with the built-in constructor when the name is a built-in one. The listener receives thatError, as in Node. The emitter'semitreports whether a listener ran, as Node's does, and the Worker cancels the event when one did. An error nothing handled reaches the parent's globalerrorevent carrying the sameError.A
oncelistener 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. The new specs fail on that branch and pass here, and the full device suite passes.
Summary by CodeRabbit