slawlor / ractor

Rust actor framework
MIT License
1.31k stars 68 forks source link

SpawnErr is misleading #164

Closed lucasmerlin closed 1 month ago

lucasmerlin commented 10 months ago

Describe the bug When a Err is returned during actor startup, the err gets wrapped with a SpawnErr::StartupPanic, while there was no panic. There should probably be a SpawnErr::StartupError or something similar, or StartupPanic should be renamed, to avoid confusion.

slawlor commented 10 months ago

This is a remnant of an earlier version of the crate, where the actor's methods were infallible. Only a panic would cause the actor to not start successfully, (or someone doing Err(_).unwrap() for example), but due to the current limitations in the panic infrastructure within Rust, you lose the context like you get with errors hence why we added support for actor functions to return an error as well.

Both are equivalent in terms of the actor (panic/error both mean it failed to start), which is why we didn't originally rename the enum variants. There's another instance of this in SupervisionEvent as well with similar syntax.

I don't consider this a bug, but something that could be renamed/adjusted in the next major version publish as it would be an API breaking change.

Karrq commented 8 months ago

I also would like to report that the StartupCancelled variant appears unused? I tried searching for references (even just a grep) but other than the Display impl it appears that the variant itself is never constructed

slawlor commented 8 months ago

Ah perhaps. There was originally a plan to use it if the handle was canceled before start completed, but perhaps the wire up point was missed