shepmaster / snafu

Easily assign underlying errors into domain-specific errors while adding context
https://docs.rs/snafu/
Apache License 2.0
1.4k stars 60 forks source link

How to box the error enum? #372

Closed kosta closed 9 months ago

kosta commented 1 year ago

I have a library where my error enum variants are getting too big (in struct size, > 128 bytes) and clippy complains.

I wanted to explore how to Box the error variant, but I'm not sure this is easily possible:

The opaque example almost looks like what I want (but with pub struct Error(pub Box<InnerError>);). This does not translate 1:1 to Boxed: E.g. it only implements From<Box<InnerError>> for Error but no From<InnerError> (but I can implement that manually).

Also, while the example shows that this compiles:

fn validate_user(user_id: i32) -> Result<(), InnerError> {
    InvalidUserSnafu { user_id }.fail()
}

I would actually like a -> Result<(), Error> which breaks the example.

Is there an easy way to do this? (i.e. without littering the already-verbose context(...) with an additional map_err(BoxedError::from))

Ideally, I would just define my Result type as pub type Result<T> = std::result::Result<T, Box<Error>>; and tell derive(snafu) to derive impl IntoError<Box<Error>> for XxxSnafu automatically (or just make the enum boxed when being told to). Do you think that is a good idea? Do you see better ways to achieve this?

shepmaster commented 1 year ago

I believe you can use source(from):

#[derive(Debug, Snafu)]
#[snafu(source(from(InnerError, Box::new)))]
pub struct Error(Box<InnerError>);
kosta commented 1 year ago

Wow, that was a quick response! Thank you! Yes, that works and that's how I implemented it now.

Just a few comments:

shepmaster commented 1 year ago

fail() still needs .map_err(Error::from)

That makes sense as ContextSelector::fail returns Result<__T, Error> — the error type is concrete and there's no opportunity for it to change. Either Into::into / From::from is appropriate (or ? which calls those implicitly).

I almost wrote Ok(Snafu{}.fail()?)

That's actually not an uncommon thing precisely for the reason of conversion. I believe that in the future you'd be able to use something like

do yeet ContextSelector { details };

Should this be exposed "more" in the user guide?

Sure, feel free to send in a PR adding to the examples section.