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

Add out of the box support for mapping custom error types that implement std::error::Error' to Box<dyn Error>` #520

Open
dcechano opened this issue Jan 10, 2025 · 2 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@dcechano
Copy link

Proposal

Problem statement

Often you need to map one Result::Err to another. This is usually straight forward but when you want to return the result as a Result<_, Box<dyn Error>> it can become cumbersome. One issue is that the compiler makes you go in circles figuring out how to do it (map_err(???)). And then when you do figure out the way the compiler likes it becomes unwieldy to constantly write .map_err(|e| e.into()) everywhere.

Motivating examples or use cases

Often you have to use code that returns a Result<_, ConcreteError> where ConcreteError implements std::error::Error. If you are in a method that returns a Result<_, Box<dyn Error>> and want to propagate these errors up the stack as Box<dyn Error> every call to the fallible function requires the error to be mapped to a Box<dyn Error>.

fn my_fallible_function() -> Result<(), Box<dyn Error>> {
    let x: Result<(), ConcreteError> = fallible_function();
    // confusingly, x.map_err(|e| Box::new(e))?; does not compile!

    x.map_err(|e| e.into())?;

    another_fallible_function().map_err(From::from)?; // I don't often see this used IRL.
    Ok(())
}

This can be improved for 2 reasons.

  1. Many people who have to do this end up fighting the compiler because the naive map_err(Box::new)? does not work. So they go in circles trying to convince the compiler to convert a Box<ConcreteError> to a Box<dyn Error>.

  2. It is cumbersome boilerplate that often exists solely to satisfy the compiler. Constantly calling map_err(|e| e.into()) can feel unidiomatic. You feel like there must be a better way. But there isn't; this is the idiomatic way. There is From::from which many don't appear to know works but even if you do know about it, it still requires constantly typing map_err(From::from).

Solution sketch

My solution is simply adding a method to the core::result::Result type that looks something like this:

impl<T, E> Result<T, E> {
    
    ...
    
    pub fn box_err(self) -> Result<T, Box<dyn Error>> 
    where E: Error {
        self.map_err(From::from)
    }
    
    ...
}

Alternatives

There are not too many alternatives I can think of other than continuing to require users to write map_err(...)?.
One possible alternative worth considering is to do something like:

impl<T, E: Error> From<Result<T, E>> for Result<T, Box<dyn Error>> {
...
}

This would allow users to just call into on the result instead of mapping the error manually. Perhaps this is a better idea because it is even more concise that the solution I suggested above.

Links and related work

This is a link to the Rust by Example book. Note that they are suggesting map_err(|e| e.into()) instead of the more idiomatic map_err(From::from), which goes to show that it this is something that the community could really use.

This is a link to the Rust users forum asking how to map to a boxed error. It is from 2019! We have been fighting the compiler about this for a while!

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@dcechano dcechano added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jan 10, 2025
@cuviper
Copy link
Member

cuviper commented Jan 10, 2025

@cuviper
Copy link
Member

cuviper commented Jan 10, 2025

That rust-by-example could be written:

fn double_first(vec: Vec<&str>) -> Result<i32> {
    Ok(vec.first()
        .ok_or(EmptyVec)?
        .parse::<i32>()? * 2)
}

Edit: actually, they address this on the next page of the book!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

2 participants