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

Using eyre with Result<_,&str> #16

Closed
benruijl opened this issue May 7, 2020 · 3 comments
Closed

Using eyre with Result<_,&str> #16

benruijl opened this issue May 7, 2020 · 3 comments

Comments

@benruijl
Copy link

benruijl commented May 7, 2020

I am trying to use eyre for unwrapping Options, for which I use ok_or and &str as the Err-type. I figured this would work with eyre, since &str and String implement std::error::Error. However, the following code does not compile:

None.ok_or("test").wrap_err("oops")

with

     = note: the method `wrap_err_with` exists but the following trait bounds were not satisfied:
             `&str: eyre::context::ext::StdError<_>`
             which is required by `std::result::Result<usize, &str>: eyre::WrapErr<usize, &str, _>`

Do I need to implement Report for &str to get this working? If so, maybe it's good to add these Report implementations for &str and String in the eyre crate itself?

@yaahc
Copy link
Collaborator

yaahc commented May 7, 2020

String and &str don't actually implement std::error::Error, they only implement Into<Box<dyn Error + ...>>, which sadly also doesn't itself implement std::error::Error because of the trait conherence rules, once we get specialization though that at least will hopefully impl Error.

I'll double check but im pretty sure it's not possible to impl From<String> for Report because it already impls From<E: Error> which would cause an orphan rules error because String could impl Error in the future, once we have negative bounds we can hopefully add impl !Error for String in std and let downstream crates depend upon String never implementing Error.

for now I think the best approach to dealing with Result<_, &str> and company is to use map

None.ok_or("test").map_err(|s| eyre!(s))

@benruijl
Copy link
Author

benruijl commented May 7, 2020

Thanks for the quick and clear answer @yaahc!

@benruijl benruijl closed this as completed May 7, 2020
@yaahc
Copy link
Collaborator

yaahc commented May 7, 2020

My pleasure, I'm also going to add this to the docs so hopefully this is clear for others in the future

/// # Wrapping Types That Don't impl `Error` (e.g. `&str` and `Box<dyn Error>`)
///
/// Due to restrictions for coherence `Report` cannot impl `From` for types that don't impl
/// `Error`. Attempts to do so will give "this type might implement Error in the future" as an
/// error. As such, `wrap_err`, which uses `From` under the hood, cannot be used to wrap these
/// types. Instead we encourage you to use the combinators provided for `Result` in `std`/`core`.
///
/// For example, instead of this:
///
/// ```rust,compile_fail
/// use std::error::Error;
/// use eyre::{WrapErr, Report};
///
/// fn wrap_example(err: Result<(), Box<dyn Error + Send + Sync + 'static>>) -> Result<(), Report> {
///     err.wrap_err("saw a downstream error")
/// }
/// ```
///
/// We encourage you to write this:
///
/// ```rust
/// use std::error::Error;
/// use eyre::{WrapErr, Report, eyre};
///
/// fn wrap_example(err: Result<(), Box<dyn Error + Send + Sync + 'static>>) -> Result<(), Report> {
///     err.map_err(|e| eyre!(e)).wrap_err("saw a downstream error")
/// }
/// ```

pksunkara pushed a commit that referenced this issue Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants