-
Notifications
You must be signed in to change notification settings - Fork 10
Reconsider top-level move semantics #50
Description
PR #48, feat: top-level move semantics, authored by @feefladder, introduced Exn::into_error(self) -> E.
That change was meant to address the move-based recovery use cases raised in #43 and, indirectly, the broader tension discussed in #40.
After looking at the use cases and the API shape more closely, I think we should reconsider whether top-level move semantics fit the intended model of exn.
The main concern is that into_error() only works for the current root error. That creates an awkward asymmetry with raise and or_raise, whose whole purpose is to add a new top-level error and push the previous exception tree into children. Once that happens, the previous error is no longer recoverable through into_error(). In practice, this creates the wrong incentive: code that wants move-based recovery may become reluctant to add context, even though preserving contextual structure is one of the crate's main strengths.
There are a few constraints here. raise really does change the semantic root; it is not just attaching metadata. Recovering a middle frame as a fresh Exn<T> is necessarily lossy with respect to discarded ancestors. Exn::new also stringifies ordinary Error::source() chains, so typed owned recovery can only ever work for explicitly stored typed frames. And more broadly, move semantics on errors seem like a niche use case compared to inspection, logging, reporting, and boundary mapping.
I see four possible directions. We can keep into_error() as-is, which is simple and convenient for the root case, but also leaves us with a root-only capability that may distort how users think about raise and or_raise. We can remove into_error(), expose Exn::into_frame(), and make Frame public enough for users to build their own owned traversal, but that leaks internals and creates a large semver burden around representation details. We can go the other way and add a real owned traversal story, such as into_frame, into_children, and Frame::downcast<E>(self) -> Result<Exn<E>, Self>, which would be much more coherent if we truly want move semantics, but is also a much larger feature commitment. Or we can decide that move semantics are simply not a first-class exn feature, and position exn errors more clearly as contextual views of failure rather than as transports for move-only recovery values.
My current leaning is the last option. If we do not want to invest in a full owned traversal design, I think keeping a top-level-only into_error() is worse than removing it. It is a narrow capability, but it nudges users in exactly the wrong direction by making root identity matter too much. If that reasoning holds, then PR #48 should likely be reverted. That is not because the implementation is wrong on its own terms, but because the capability it introduced may not fit the long-term model of the crate.
I would especially like feedback on two questions: whether move-based recovery is common enough in real exn usage to justify a larger owned traversal design, and whether it is better to remove into_error() now, while the change is not released yet.