Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Beiri22 opened this issue Feb 9, 2024 · 11 comments
Closed

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

Beiri22 opened this issue Feb 9, 2024 · 11 comments
Labels
S-question Status: Further information is requested

Comments

@Beiri22
Copy link

Beiri22 commented Feb 9, 2024

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
Copy link
Contributor

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
Copy link
Author

Beiri22 commented Feb 12, 2024

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
Copy link
Contributor

Could you please post some working example code?

@Beiri22
Copy link
Author

Beiri22 commented Feb 12, 2024

 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
Copy link
Contributor

LeoniePhiline commented Feb 19, 2024

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(())
}

@LeoniePhiline LeoniePhiline added the S-question Status: Further information is requested label Feb 26, 2024
@Beiri22
Copy link
Author

Beiri22 commented Apr 6, 2024

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
Copy link
Contributor

LeoniePhiline commented Apr 6, 2024

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
Copy link

pickx commented May 23, 2024

@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

  • E1: std::error::Error + Send + Sync + 'static
  • E2: std::error::Error + Send + Sync + 'static

...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
Copy link
Author

Beiri22 commented May 24, 2024

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
Copy link
Contributor

@Beiri22 Are you okay with having this issue closed?

@Beiri22 Beiri22 closed this as completed Jun 29, 2024
@Beiri22
Copy link
Author

Beiri22 commented Jun 29, 2024

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-question Status: Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants