Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy over the error from inside workers to the Worker error event? #7596

Open
domenic opened this issue Feb 9, 2022 · 3 comments
Open

Copy over the error from inside workers to the Worker error event? #7596

domenic opened this issue Feb 9, 2022 · 3 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: error reporting topic: serialize and transfer topic: workers

Comments

@domenic
Copy link
Member

domenic commented Feb 9, 2022

When an uncaught exception occurs inside a worker, it fires:

  • An ErrorEvent "inside", on the WorkerGlobalScope object. The event.error property contains the exception value.

  • An ErrorEvent "outside", on the Worker object. The event.error property is null.

(Relevant spec section)

This made sense, back before we had a way to serialize exceptions across thread boundaries. However, #4665 introduced that ability! So maybe we should consider setting the "outside" error property to a clone of the "inside" error property?

IMO this has a few prerequisites before I'd be fully comfortable with it:

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: workers topic: serialize and transfer labels Feb 9, 2022
@annevk
Copy link
Member

annevk commented Feb 15, 2022

cc @asutherland

@asutherland
Copy link

This definitely seems reasonable given the new serialization support. Thank you!

Offtopic-ish Aside: It might also be nice to consider improving the ergonomics around unhandledrejection so that they also could propagate to the Worker binding. I feel like the last time I looked at this it was necessary to add a handler in the worker that would then naively throw the rejection in order to get propagation. Propagation has been desirable at least in our own internal system code support infrastructure so that tests can fail with an explicit failure instead of mysteriously timing out when some very broken thing happens inside the worker, derailing it.

@RealAlphabet
Copy link

This definitely seems reasonable given the new serialization support. Thank you!

Offtopic-ish Aside: It might also be nice to consider improving the ergonomics around unhandledrejection so that they also could propagate to the Worker binding. I feel like the last time I looked at this it was necessary to add a handler in the worker that would then naively throw the rejection in order to get propagation. Propagation has been desirable at least in our own internal system code support infrastructure so that tests can fail with an explicit failure instead of mysteriously timing out when some very broken thing happens inside the worker, derailing it.

Would it be possible to take advantage of this to expose more information about the PromiseRejectionEvent event ? The same information currently exposed by ErrorEvent.

ErrorEvent.filename Read only
A string containing the name of the script file in which the error occurred.

ErrorEvent.lineno Read only
An integer containing the line number of the script file on which the error occurred.

ErrorEvent.colno Read only
An integer containing the column number of the script file on which the error occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: error reporting topic: serialize and transfer topic: workers
Development

No branches or pull requests

4 participants