Untyped error?

[tisonkun]

In @Byron 's PR of vendoring exn (GitoxideLabs/gitoxide#2352), it introduces an Untype structure as the default type param of Exn<E>:

/// A marker to show that type information is not available,
/// while storing all extractable information about the erased type.
/// It's the default type for [Exn].
pub struct Untyped(Box<dyn Error + Send + Sync + 'static>);

impl fmt::Display for Untyped {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        std::fmt::Display::fmt(&self.0, f)
    }
}

impl fmt::Debug for Untyped {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        std::fmt::Debug::fmt(&self.0, f)
    }
}

impl Error for Untyped {}

pub struct Exn<E: std::error::Error + Send + Sync + 'static = Untyped> {
    frame: Box<Frame>,
    phantom: PhantomData<E>,
}

impl<E: Error + Send + Sync + 'static> Exn<E> {
    /// Erase the type of this instance and turn it into a bare `Exn`.
    pub fn erased(self) -> Exn {
        Exn {
            frame: self.frame,
            phantom: Default::default(),
        }
    }
}

Another related extension is that gix-error provides a type-erased Error type that works like anyhow::Error:

/// An error type that wraps an inner type-erased boxed `std::error::Error` or an `Exn` frame.
///
/// In that, it's similar to `anyhow`, but with support for tracking the call site and trees of errors.
///
/// # Warning: `source()` information is stringified and type-erased
///
/// All `source()` values when created with [`Error::from_error()`] are turned into frames,
/// but lose their type information completely.
/// This is because they are only seen as reference and thus can't be stored.
pub struct Error {
    inner: error::Inner,
}

pub(super) enum Inner {
    ExnAsError(Box<exn::Frame>),
    Exn(Box<exn::Frame>),
}

impl Inner {
    fn as_frame(&self) -> &exn::Frame {
        match self {
            Inner::ExnAsError(f) | Inner::Exn(f) => f,
        }
    }
}

impl Error {
    /// Create a new instance representing the given `error`.
    #[track_caller]
    pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self {
        Error {
            inner: Inner::ExnAsError(Exn::new(error).into()),
        }
    }
}

impl std::fmt::Display for Error {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match &self.inner {
            Inner::ExnAsError(err) => std::fmt::Display::fmt(err.as_error(), f),
            Inner::Exn(frame) => std::fmt::Display::fmt(frame, f),
        }
    }
}

impl std::fmt::Debug for Error {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        match &self.inner {
            Inner::ExnAsError(err) => std::fmt::Debug::fmt(err.as_error(), f),
            Inner::Exn(frame) => std::fmt::Debug::fmt(frame, f),
        }
    }
}

impl std::error::Error for Error {
    /// Return the first source of an [Exn] error, or the source of a boxed error.
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        match &self.inner {
            Inner::ExnAsError(frame) | Inner::Exn(frame) => {
                frame.children().first().map(exn::Frame::as_error_no_send_sync)
            }
        }
    }
}

impl<E> From<Exn<E>> for Error
where
    E: std::error::Error + Send + Sync + 'static,
{
    fn from(err: Exn<E>) -> Self {
        Error {
            inner: Inner::Exn(err.into()),
        }
    }
}

Created this issue to track these ideas. And I'd appreciate it if @Byron could share some design consideration and their typical use cases.

cc @andylokandy, we may consider whether or not include these designs in the exn upstream. So far, I can somehow understand their purpose, but I hesitate about the concrete API and names.

1. Putting Untyped as the default type param seems a bit too extreme;
2. The tree-structure and StdError::source's mismatch is still something our users must understand;
3. Once we have both typed and untyped branches, we need some guidelines and best practices to use them in different scenarios.

Currently, to apply the philosophy of designing for error across different modules as described in our blog, the typed branch is preferable. The untyped branch is mainly for convenience. But I'm not yet convinced that putting such two branches into one crate a good way to go.

cc @80Ltrumpet in case you're interested in this topic and have some spare time to consider the questions here.

[andylokandy]

The tree-structure and StdError::source's mismatch is still something our users must understand;

This is not applied to the use case in this issue--the error in Exn does not have a source at all.

The untyped branch is mainly for convenience.

Plus for progressively migrating from untyped approach to typed approach.

Once we have both typed and untyped branches, we need some guidelines and best practices to use them in different scenarios.

I'll concern if allowing untyped branches encourages postponing adding context, circling back to anyhow.

[Byron]

Thanks for starting this issue! I'd love to see you come up with something better than what I came up with in a jiffy, just to make it work right now.

The untyped branch is mainly for convenience.

Plus for progressively migrating from untyped approach to typed approach.

My usecase is delegates or callbacks. Thanks to exn they don't need to be typed at all and it's fine to type-erase them. Previously one would have to use Box, but that always looked too ugly and was unergonomic to use. Generally, I want to avoid generics when I can, and having something untyped is really useful.

Besides, I found myself liking Exn in Result<T, Exn> very much, it's very convenient, and kind of understandable that it's not Error, but it's also not an 'exception' - it's something in-between :D .

[Byron]

Warning: the fn erased() implementation is buggy, it should be something like this:

    /// Erase the type of this instance and turn it into a bare `Exn`.
    pub fn erased(self) -> Exn {
        let untyped_frame = {
            let Frame {
                error,
                location,
                children,
            } = *self.frame;
            // Unfortunately, we have to double-box here.
            // TODO: figure out tricks to make this unnecessary.
            let error = Untyped(error);
            Frame {
                error: Box::new(error),
                location,
                children,
            }
        };
        Exn {
            frame: Box::new(untyped_frame),
            phantom: Default::default(),
        }
    }

The fix will be merged today I hope.

[80Ltrumpet]

I'll lend my two cents.

I agree that type-erasure should only be considered a convenience. Consider this toy example:

use std::{io, fmt};
use exn::ErrorExt;

fn foo() -> exn::Result<(), fmt::Error> {
    Err(fmt::Error.raise())
}

fn bar() -> exn::Result<(), io::Error> {
    Err(io::Error::other("bar").raise())
}

// What should the `Err` type be?
fn main() -> Result<(), ???> {
    foo()?;
    bar()?;
    Ok(())
}

My solution in #34 was to provide an impl From like so (using names more applicable to this issue):

use std::{error::Error, fmt::{Debug, Display}};
use exn::Exn;

pub struct ExnAny(Box<dyn Error + Send + Sync + 'static>);

// These just delegate to the `Box`ed type.
impl Debug for ExnAny { … }
impl Display for ExnAny { … }

impl Error for ExnAny {}

impl<T> From<Exn<T>> for ExnAny
where
    T: Error + Send + Sync + 'static,
{
    fn from(exn: Exn<T>) -> Self {
        Self(Box::new(ExnError(exn)))
    }
}

struct ExnError<T>(Exn<T>)
where
    T: Error + Send + Sync + 'static;

// These just delegate to `Exn`.
impl<T> Debug for ExnError<T> where … { … }
impl<T> Display for ExnError<T> where … { … }

impl<T> Error for ExnError<T> where T: Error + Send + Sync + 'static {}

In the original toy example, ??? becomes exn::ExnAny, and the application Debugs out the originating Exn as you would expect.

Note that ExnError only exists to newtype Exn into an impl Error for the purpose of creating a trait object. Ideally, we could just specify Box<dyn Debug + Display + …> and avoid ExnError altogether.

In my opinion, it is unnecessary to try to force Exn into an Error shape—i.e., don't bother with Error::source at all—but perhaps I am not familiar with the use-case for it.

[Byron]

Most applications I know use anyhow which relies on source() to be implemented to show errors chains like

    root

    Caused by:
        0: E2
        1: E1
        2: E1-2
        3: E1-3
        4: E1c1-1
        5: E1c1-2
        6: E1c2-1
        7: E1c2-2

[80Ltrumpet]

Most applications I know use anyhow which relies on source() to be implemented […]

In that case, a type-erased Exn could simply provide a Display implementation that writes the tree (i.e., by delegating to Exn's Debug implementation). Conveniently, anyhow properly indents multi-line Display representations.

Supposing we want to explicitly interoperate with anyhow (and reusing ExnError from my previous comment), you can do something like this:

use std::{error::Error, fmt::{self, Debug, Display, Formatter}};
use anyhow::Context;
use exn::{ErrorExt, Exn};

struct ExnAnyhow(Box<dyn Error + Send + Sync + 'static>);

impl Debug for ExnAnyhow {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        Debug::fmt(&*self.0, f)
    }
}

impl Display for ExnAnyhow {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        // Not a typo! We always want the entire `Exn` tree to show up.
        Debug::fmt(&*self.0, f)
    }
}

impl Error for ExnAnyhow {}

impl<T> From<Exn<T>> for ExnAnyhow
where
    T: Error + Send + Sync + 'static,
{
    fn from(exn: Exn<T>) -> Self {
        Self(Box::new(ExnError(exn)))
    }
}

fn foo() -> Result<(), ExnAnyhow> {
    let e1 = std::io::Error::other("child").raise();
    let e2 = e1.raise(std::io::Error::other("parent"));
    Err(e2.into())
}

fn main() -> anyhow::Result<()> {
    foo().context("anyhow context")?;
    Ok(())
}

This prints:

Error: anyhow context

Caused by:
    parent, at example.rs:35:17
    |
    |-> child, at example.rs:34:45

[andylokandy]

What do you guys think of #29? It'll allow ? on Exn in a function returning anyhow::Result<T> or Result<T, Box<dyn Error>>.

[80Ltrumpet]

How do you guys think of #29? It'll allow ? on Exn in a function returning anyhow::Result<T> or Result<T, Box<dyn Error>>.

I think it's a reasonable default, as long as it's clear to users that the Into<Box<dyn Error + …>> implementations will only include the first child frame in the source chain. For users who want to have their cake and eat it, too, they can roll their own solution. Whether you'd like to also provide an example for that is, of course, up to y'all.

[Byron]

How does downcasting work if the errors aren't officially part of the error chain? Thus, it's not only about Display.

[80Ltrumpet]

How does downcasting work if the errors aren't officially part of the error chain? Thus, it's not only about Display.

I'm not sure if that was a question for me, but I believe we're concerned about different use-cases.