Helper for passing extra context to Errors

libs
1

When errors implement From, propagation of errors is very easy and terse:

do_this(arg1)?.do_that(arg2)?;

However, the From impl is context-free, and some errors aren't informative on their own (the io::Error without a filename!)

anyhow helps here by having a .context() method, but that works only for anyhow's own error type. For libraries using specific enum Error (thiserror-style) this isn't as neat:

do_this(arg1).map_err(|e| Error::new(e, arg1))?.do_that(b).map_err(|e| Error::new(e, arg2))?;

And something like .context() would require extra traits and bespoke implementations.

I wonder if the standard library could provide some universal helper method to cut down on the syntactic noise of map_err(|_|).

For example, there's option.zip(new_val) that makes Some((previous, new_val)). What if Result had .zip? It would be possible to implement From<(Error, Context)> for CustomError, even by boilerplate-generating libraries, so code like this could work:

do_this(arg1).zip(arg1)?.do_that(b).zip(arg2)?;

For example imagine impl From<(io::Error, PathBuf)> for ErrorWithPath + zip:

fs::File::open(&path).zip(path)?

zip is not the best name for it, but maybe a better-named similar pattern is possible?

7 Likes
Sum types in product types
2

Some prior art I feel ought to be mentioned is @shepmaster's snafu, which covers both sides of the anyhow/thiserror divide but prefers a style that is thiserror-style plus anyhow-style helper methods, including .context().

1 Like
3

I really like the shape of the zip idea because it means that the correct type of error-with-more-context can be determined by the combination of the existing error type and the type of context provided — there's a lot of potential for error types making it ergonomic to use them well.

However, I worry that there might be quirks lurking in the trait coherence rules and type inference, such that it might be better to have a dedicated trait for the purpose rather than From in particular — so whether or not that specifically is true, it would be good to test the limits imposed by the chosen trait. For example, what happens if you try to blanket implement for a concrete context type and all error types? For a concrete error and all contexts? I'm not confident enough in my understanding of the trait coherence rules to know the answers already, and probably few people are, so concrete examples of what is and isn't possible would probably make a good component of a future RFC.

4

if the method literally is Err(v).zip_thing(v2) to Err((v, v2)), then I think an appropriate name is zip_err, to go along with map_err that we already have

4 Likes
5

I love this, but what happens if I zip twice?

myresult.zip_err(additional_info1).zip_err(additional_info2)

In my mind ideally it should result in a triple Result<T, (OriginalErr, Info1, Info2)>, but then writing the method becomes very complicated (not impossible, but would require complicated type level gymnastics), with poor type error messages (unless it is special cased somehow)

But with the vanilla idea it would be Result<T, ((OriginalErr, Info1), Info2)>, which is probably not too bad.

This probably showcases the need for either variadic generics or type level lists (like frunk's HList, which semantically are the same as tuples, but that you can pattern match at the type level with HCons and HNil)

6

Also, should pairs (Error, Info) be used here, or another type that conveys what it is used for?

If it were something like ErrorContext<OriginalError, Info>, it would quickly be too noisy if you nested zips. So it should be pairs probably.

7

It actually works fine and is super flexible.

The trait is in the form of From<T> for MyCustomErrorType, and because a crate-local type is on the right side of for, the From part can be almost anything. The only conflicting blanket implementation is From<MyCustomErrorType> for MyCustomErrorType, so as long as you use a tuple in From<(T, U)>, it can contain anything.

I chose From for integration with the ? operator, because result.zip_err(x)? will automatically apply ErrorInFunctionReturnType::from((err, x)).

Admittedly, (re)use of a tuple for this purpose is "clever". OTOH From is already used for error conversion, and another trait would need to be very From-like anyway:

impl ContextFrom<E, C> for MyCustomError {
   fn from_context(error: E, context: C) -> Self { … }
}

It doesn't use associated types, because you may want to convert (io::Error, PathBuf) to MyReadError or MyWriteError or CantFindConfigFileError and so on.

It does rely on the function's error type to be specific, but that's nothing new. When the error type is generic/unknown Rust already needs hints for the From conversion.

Rust generally doesn't flatten tuples. .zip behaves in the same nested way on Option and Iterator, so even though it's non-ideal to have lisp-looking nested list, I think zipping of errors should stick to that for simplicity and consistency. In practice people would probably use .zip((info1, info2)) or proper map_err if there's complex context to add.

1 Like
8

Okay, but for example, what if you want to produce a different library's error type, because you're in the position of adapting to an existing type instead of adapting from an existing type? In general, I'm not saying this is going to be a problem; I'm saying the bounds of what will be possible given the design should, eventually, be thoroughly described in the proposal.

9

Ah, in this case indeed:

impl From<(MyError, T)> for io::Error

is not allowed, but:

impl ContextFrom<MyError, T> for io::Error

is allowed, because trait coherence is defined in terms of type arguments in the trait, and not type arguments in tuples (they don't get fundamental type exception).

2 Likes
10

When I am trying to add context to an error, often I'm trying to add two things: the object I was operating on, and what I was trying to do with it. A common pattern would be:

let file = open(config_file).zip_err(format!("Opening config file '{config_file}'"))?;

The problem with that is that the context gets prepared in the non-error case as well. map_err bypasses this by taking a closure, as does with_context if using anyhow.

How would you do that using this pattern?

3 Likes
11

Here's how I tend to do it already: defer the expensive parts until the implicit into and/or until you actually display things. And From<(SrcErrWithContext, &CheapThing)> is often a part of that.

Adapted from a project:

pub struct FileReadError {
    path: PathBuf,
    kind: FileReadErrorKind,
}

enum FileReadErrorKind {
    Open(io::Error), // io::Error is too general, so these
    Read(io::Error), // supply more context
    Parse(FileLineError),
}

The expensive parts:

impl From<(FileReadErrorKind, &Path)> for FileReadError {
    fn from((kind, path): (FileReadErrorKind, &Path)) -> Self {
        let path = path.to_owned();
        Self { path, kind }
    }
}

impl Error for FileReadError {
    fn source(&self) -> Option<&(dyn Error + 'static)> {
        // (return the io::Error etc .. xor include them in Display below)
    }
}

impl fmt::Display for FileReadError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        use FileReadErrorKind as Kind;
        match self.kind {
            Kind::Open(_) => write!(f, "could not open {} for reading", self.path.display()),
            // ...
        }
    }
}

What using the infrastructure looks like:

pub fn open(path: &Path) -> Result<Self, FileReadError> {
    use FileReadErrorKind as Kind;
    // Today / with zip_err
    let file = File::open(path).map_err(|e| (Kind::Open(e), path))?;
    let file = File::open(path).map_err(Kind::Open).zip_err(path)?;
    // ...

    // Today / with zip_err
    this.read_inner(file).map_err(|e| (e, path))?;
    this.read_inner(file).zip_err(path)?;

    Ok(this)
}
fn read_inner(&mut self, mut file: File) -> Result<(), FileReadErrorKind> {
    // ...
}
1 Like
12

I imagine this could be adapted to use the same pattern as with_context

let file = open(config_file)
  .with_zip_err(|| format!("Opening config file '{config_file}'"))?;

with a thiserror style Error of

#[derive(Error, Debug)]
enum Error {
    #[error("IO Error of: {0} with context of {1:?}")]
    Io(#[from] std::io::Error, #[context] Option<String>)
}
13

well, you could just do the formatting in the From implementation:

struct MyError {
    io_err: io::Error,
    context_str: String,
}

impl From<(io::Error, fmt::Arguments<'_>)> for MyError {
    fn from((io_err, context): (io::Error, fmt::Arguments<'_>)) -> Self {
        Self { io_err, context_str: context.to_string() }  // does the expensive formatting here
    }
}

// constructing fmt::Arguments is cheap, no lambda function needed
let file = open(&path_to_read).zip_err(format_args!("trying to open {path_to_read}"))?;
1 Like
14

This is the path snafu takes, although it uses named, macro-generated types rather than tuples.

1 Like
15

Option has zip_with, so Result could have zip_err_with too.

Deferring work until From/Into sounds like a good idea. There's even a non-allocating format_args! that can be used to defer string formatting, while remaining in the same scope. From<(E, impl Display)> could handle all string types + format_args!.

1 Like
16

That works if every context is going to be converted to a String, but I imagine there would be a desire to support non-string contexts as well.

Code Example 
let entry = query!(include_str!("fetch_by_id.sql"))
    .bind(id)
    .fetch_optional(&pool)
    .await
    .zip_err_with(|| SqlContext::Fetch(id.into()))?;

let result = query!(include_str!("update_value.sql"))
    .bind(id)
    .bind(value)
    .execute(&pool)
    .await
    .zip_err_with(|| SqlContext::Update(id.into(), value))?;

#[derive(Context, Debug)]
enum SqlContext {
    #[context("Failed to read id: {0}")]
    Fetch(InlineableString),
    #[context("Failed to update id: {0} to value of {1}")]
    Update(InlineableString, isize),
}

#[derive(Error, Debug)]
enum Error {
    #[error("sql error occurred {0}")]
    Sql(#[from] sqlx::Error, #[context] Option<SqlContext>),
}

I feel if zip_err is added, than zip_err_with should also be added. Whether or not the From implementation includes an implicit clone is up to the different error crate developers.

1 Like
17

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.