search

GoogleChromeLabs / comlink

Comlink makes WebWorkers enjoyable.
Apache License 2.0
11.37k stars 390 forks source link

Use `serialize-error` for error serialization #594

Open safareli opened 2 years ago

safareli commented 2 years ago

If you are using custom error classes with custom properties or even more or less standard cause then you would loose some data here: https://github.com/GoogleChromeLabs/comlink/blob/4ba8162f6c28fb1bf53b491565ef9a3ae42b72d3/src/comlink.ts#L249-L256

Instead I'm proposing to use serialize-error that works better then what we have currently.

I can create PR if this is approved.

Meanwhile you can do this as a workaround

benjamind commented 1 year ago

Given the workaround is sufficient here, and adding the serialization would for most folks be unnecessary additional payload do you think this is actually worth adding to the library, or should we just document the workaround for those folk that need it?

safareli commented 1 year ago

The workaround depends on internal api and needs unsafe type casts. it would be nice to just use serialize-error but if adding dependency is a problem then making API of swapping out transferHandlers in a nicer way would be one option.

Maybe some sort of plugin api could be implemented here. and there would be a plugin that uses serialize-error for errors

benjamind commented 1 year ago

I guess I missed the usage of an internal api, which part uses that?

If you want to put up a PR to use serialize-error so we can compare payload impact that'd be the best path forward here. Besides that I think we would need more indication of this being a consistent problem for users to determine if its worth addng the additional payload for this.

modiho commented 3 months ago

I just want to add that I just spent some time trying to find out why my error messages did not include the error cause. I did not realize at first that the error crossed the worker - main thread boundary or that info might be discarded at that boundary.