Skip to content

Add new API try_downcast_inner to std::io::Error #57

Closed

Description

Proposal

Add new API try_downcast_inner to std::io::Error:

impl std::io::Error {
    fn try_downcast_inner<E: Error + Send + Sync + 'static>(self) -> Result<Box<E>, Self>;
}

Problem statement

Existing APIs requires two separate calls to obtain the raw os error or the inner error and they both return Option.
There's no way to avoid unwrap/expect if we want to cover every possible corner case here, e.g., both of them somehow returns None.

This is impossible without expect/unwrap because std::io::Error::into_inner takes the error by value and returns Option<Box<dyn Error + Send + Sync>> instead of Result<Box<dyn Error + Send + Sync>, Self>.

Currently, we would have to do this to downcast the inner error:

(Adapted from cargo-bins/cargo-binstall#180)

#[derive(Debug)]
#[derive(thiserror::Error)]
enum E {
    #[error(transparent)]
    Io(std::io::Error),

    #[error("...")]
    SomeOtherVariant,
}

impl From<std::io::Error> for E {
    fn from(err: std::io::Error) -> E {
        if err.get_ref().is_some() {
            let kind = err.kind();

            let inner = err
                .into_inner()
                .expect("err.get_ref() returns Some, so err.into_inner() should also return Some");

            inner
                .downcast()
                .map(|b| *b)
                .unwrap_or_else(|err| E::Io(std::io::Error::new(kind, err)))
        } else {
            E::Io(err)
        }
    }
}

This has only 1 expect/unwrap, but it uses std::io::Error::new, which uses Box::new internally.

Another way to implement it would be:

impl From<std::io::Error> for E {
    fn from(err: std::io::Error) -> E {
        let is_E = err
            .get_ref()
            .map(|e| e.is::<E>())
            .unwrap_or_default();

        if is_E {
            let inner = err
                .into_inner()
                .expect("err.get_ref() returns Some, so err.into_inner() should also return Some");

            inner
                .downcast()
                .map(|b| *b)
                .expect("e.is::<E>() returns true, so this must succeed")
        } else {
            E::Io(err)
        }
    }
}

This saves the call to std::io::Error::new, but adds another unwrap/expect.

Thus, I propose to add another API to simplify the downcasting while avoiding unwrap/expect, which is std::io::Error::try_downcast_inner.

It returns Result<Box<E>, Self>, and when the downcast fails, it simply return the original std::io::Error.

Motivation, use-cases

This makes downcasting much easier:

#[derive(Debug)]
#[derive(thiserror::Error)]
enum E {
    #[error(transparent)]
    Io(std::io::Error),

    #[error("...")]
    SomeOtherVariant,
}

impl From<std::io::Error> for E {
    fn from(err: std::io::Error) -> E {
        err.try_downcast_inner::<E>()
            .map(|b| *b)
            .unwrap_or_else(E::Io)
    }
}

Solution sketches

The first solution I came to mind with is:

impl std::io::Error {
    fn into_underlying_err(self) -> Cause;
    fn from_underlying_err(cause: Cause) -> Self;
}

#[non_exhaustive]
enum Cause {
    RawOsError(i32),
    InnerError(Box<dyn Error + Send + Sync>),
}

Then I realized that this is too complex and exposes too much details of std::io::Error.

The second solution:

impl std::io::Error {
    fn into_inner2(self) -> Result<Box<dyn Error + Send + Sync>, Self>
}

The problem with this solution is that it creates confusion by introducing a new function with name very similar to existing one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    ACP-acceptedAPI Change Proposal is accepted (seconded with no objections)T-libs-apiapi-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions