eyre-rs / eyre

A trait object based error handling type for easy idiomatic error handling and reporting in Rust applications
Apache License 2.0
1.35k stars 63 forks source link

and_then() that automatically converts error type to Report? #157

Closed Beiri22 closed 2 months ago

Beiri22 commented 7 months ago

When chaining Result's and_then() calls, it seems to not work with different error types. I would like to see some kind of easy way like a method and_then_eyre that takes the error supplied and convert it into a Report. Something like:

  fn and_then_eyre<E: Error + Send + Sync, U, F: FnOnce(T) -> Result<U, E>>(self, op: F) -> std::result::Result<U, Report> {
        self.and_then(|x| op(x).map_err(Report::new))
    }
    fn and_then_dyn_eyre<E: Error + Send + Sync + ?Sized, U, F: FnOnce(T) -> Result<U, Box<E>>>(self, op: F) -> std::result::Result<U, Report> {
        self.and_then(|x| op(x).map_err(|e| eyre!(e)))
    }
LeoniePhiline commented 6 months ago

Wouldn't you then alternate your and_then calls with .wrap_err("error message")? calls?

There's already plenty ways to produce reports from other errors.

Beiri22 commented 6 months ago

You cannot alternate, when and_then crashes. and_then is defined as pub fn and_then<U, F: FnOnce(T) -> Result<U, E>>(self, op: F) -> Result<U, E> There is only one error type E. When having a Result<_, Report> and_then only applies to functions returning an Result<_, Report>. If you try to chain a like IoResult with IoError, you cannot do. You get: note: expected enum Result<_, ErrReport> found enum Result<std::string::String, std::io::Error>

And Option has option.ok_or_eyre()

LeoniePhiline commented 6 months ago

Could you please post some working example code?

Beiri22 commented 6 months ago
 let cookie_file = Some("path"); // None
 let cookie_jar = cookie_file
        .ok_or_eyre("")
        .and_then(File::open);
  // more chained methods, later a single default for all error cases.
LeoniePhiline commented 6 months ago

Why would you chain on the Result, rather than propagate the error?

E.g.

use std::fs::File;

use eyre::OptionExt;

fn main() -> eyre::Result<()> {
    let cookie_file = Some("path"); // None
    let _cookie_jar = File::open(cookie_file.ok_or_eyre("must provide cookie file path")?)?;

    Ok(())
}

If you require chaining because your simplified code is actually a stream or iterator combinator, then convert your custom Result::Err(_) to Report as usual within the closure:

use std::fs::File;

use eyre::{OptionExt, WrapErr};

fn main() -> eyre::Result<()> {
    let cookie_file = Some("path"); // None
    let _cookie_jar = cookie_file
        .ok_or_eyre("must provide cookie file path")
        .and_then(|path| File::open(path).wrap_err("failed to open cookie jar"))?;

    Ok(())
}

However, I would strongly advise to propagate errors where they occur, rather than dragging them through long chains. The latter gets quite opaque quickly.

E.g. in the example, the error might be created at ok_or_eyre, but is only propagated later at the ?. This is not entirely obvious at first glance, and even less when code gets more complex.

In a real life application, I would always propagate immediately:

use std::{fs::File, path::Path};

use eyre::{OptionExt, WrapErr};

fn main() -> eyre::Result<()> {
    // External arg, not known in advance.
    let maybe_cookie_file_path = Some(Path::new("path")); // None

    // We either bail or continue on the happy path, with `path: &Path`.
    let path = maybe_cookie_file_path.ok_or_eyre("must provide cookie file path")?;

    // Again, bail (propagating the error to the caller)
    // or continue on the happy path with `cookie_jar: File`.
    let cookie_jar = File::open(path).wrap_err("failed to open cookie jar")?;

    // Do something with `cookie_jar`.
    todo!();

    Ok(())
}
Beiri22 commented 5 months ago

In this case for any kind of error in this chain a (the same) default should apply.

.and_then(|path| File::open(path).wrap_err("failed to open cookie jar"))?;

The idea was to make this expression a little bit boilerplate-ly. If you prefer the explicit version, then we'd better close the issue.

LeoniePhiline commented 5 months ago

I see what you mean. I think what your are really looking for is try blocks. These are currently unstable.

You can read a succinct overview in the unstable book and follow the tracking issue.

This would allow you to bail using ? in a chain and map/wrap the try block’s result err with eyre.

In stable rust, you could, as a workaround, refactor your combinator chain into a function returning plain Result<T, Box<dyn Error>> and then use eyre on the function call’s return value.

Alternatively, following your initial design, you could implement the methods you proposed on a ResultExt trait.

I can’t make the call for if such an extension trait should be part of eyre. However, you can safely assume that maintainers of this crate are going to have read your proposal and might chime in to the discussion with their own thoughts on the matter.

Also you might like to consider joining the eyre Discord for some closer contact. Please don’t feel discouraged, I might just misunderstand you, and my role on this team is very small. I’m in no position to accept or reject your proposal. I’ve just been trying to think along.

pickx commented 3 months ago

@Beiri22 you can try defining a trait like

trait AndThenEyre<T, E1> {
    fn and_then_eyre<U, E2, F>(self, op: F) -> Result<U, eyre::Report>
    where
        F: FnOnce(T) -> Result<U, E2>;
}

impl<T, E1> AndThenEyre<T, E1> for Result<T, E1> {
    fn and_then_eyre<U, E2, F>(self, op: F) -> Result<U, eyre::Report>
    where
        F: FnOnce(T) -> Result<U, E2>,
    {
        match self {
            Ok(t) => match op(t) {
                Ok(u) => Ok(u),
                Err(e2) => Err(eyre::Report::from(e2)),
            },
            Err(e1) => Err(eyre::Report::from(e1)),
        }
    }
}

this may sound useful (I sure wish I had something like this), but there's a fundamental limitation here.

this would require you to add the bounds

...and the problem here is that eyre::Report does not (and as I understand it, cannot) implement std::error::Error. so:

  1. you can't call this on eyre::Result
  2. you can't call this with any op that returns eyre::Result
  3. even if you call this on a Result<T, E1> with an op that returns Result<U, E2> where both E1 and E2 implement Error, you now have an eyre::Result so you can never chain another and_then_eyre

so not only does this limit the usefulness of this function, it just makes this API weird in the context of a crate that wants to encourage you to use eyre::Report.

Beiri22 commented 3 months ago

Yes, its difficult to achive. In my case I used the functions of the original post put in an extention trait to result:

pub trait ResultExt<T> {
    fn and_then_eyre<E: Error + Send + Sync + 'static, U, F: FnOnce(T) -> Result<U, E>>(self, op: F) -> std::result::Result<U, Report>;
    fn and_then_dyn_eyre<E: Error + Send + Sync + ?Sized + 'static, U, F: FnOnce(T) -> Result<U, Box<E>>>(self, op: F) -> std::result::Result<U, Report>;
}

to make it work for my specific case.

LeoniePhiline commented 2 months ago

@Beiri22 Are you okay with having this issue closed?

Beiri22 commented 2 months ago

sure