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

Feature request/thoughts: Report<E> with known “base type” #392

Open
Porges opened this issue Aug 1, 2024 · 2 comments
Open

Feature request/thoughts: Report<E> with known “base type” #392

Porges opened this issue Aug 1, 2024 · 2 comments

Comments

@Porges
Copy link
Contributor

Porges commented Aug 1, 2024

I find myself wanting a version of Report with a well-known base type.

At the moment, as soon as you add some kind of dynamic information (context or source code) to an error you lose information about what the errors could be since everything ends up as Report. This unfortunately reduces us to dynamic downcasting as in most languages with exception-based errors. It's possible to bake in source code information to your errors sometimes, but not always when dealing with error types from other libraries unless you want to re-wrap them all as more cases of your error type (and this gets uglier if error types are parameterized).

What I’m picturing is a Report<E> which is guaranteed to always be downcastable to E in addition to other possible dynamic downcasts. (Sample implementation below).


The tricky part I ran into is that you really want to be able to convert from Report<EFrom> to Report<ETo> so that you are able to convert error types. I don't think this can be accomplished by using the public API while retaining all context. You can downcast to EFrom and convert it, but at that point you've thrown all the context away. Maybe something like this on Context<T,E> would work?

// Locates first error of type E and replaces it with the result of 
// applying `f`.
fn map_within_context<E, E2>(self, f: F) -> Report
where
  F: FnOnce(E) -> E2,
  E: Display + Send + Sync + 'static,
  E2: Display + Send + Sync + 'static;

Demo of what I attemped:

use std::fmt::{Debug, Display};

use miette::Diagnostic;

// --- Basic definitions: ---

pub struct Report<E> {
    inner: miette::Report,
    phantom: std::marker::PhantomData<E>,
}

impl<E: Diagnostic + Send + Sync + 'static> Report<E> {
    pub fn new(error: E) -> Self {
        Self {
            inner: miette::Report::new(error),
            phantom: std::marker::PhantomData,
        }
    }
}

impl<E> From<E> for Report<E>
where
    E: Diagnostic + Send + Sync + 'static,
{
    fn from(error: E) -> Self {
        Self::new(error)
    }
}

// --- Inherit standard miette behaviours: ---

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

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

// --- Adding context preserves the underlying error type: ---

impl<E> Report<E> {
    pub fn context(self, msg: impl Display + Send + Sync + 'static) -> Self {
        Self {
            inner: self.inner.context(msg),
            phantom: std::marker::PhantomData,
        }
    }

    pub fn with_source_code(self, source_code: impl miette::SourceCode + 'static) -> Self {
        Self {
            inner: self.inner.with_source_code(source_code),
            phantom: std::marker::PhantomData,
        }
    }
}

// --- These downcasts are guaranteed to work: ---

impl<E: Display + Debug + Send + Sync + 'static> Report<E> {
    pub fn downcast_base(self) -> E {
        // UNWRAP: guaranteed by type parameter
        self.inner.downcast().unwrap()
    }

    pub fn downcast_base_ref(&self) -> &E {
        // UNWRAP: guaranteed by type parameter
        self.inner.downcast_ref().unwrap()
    }

    pub fn downcast_base_mut(&mut self) -> &mut E {
        // UNWRAP: guaranteed by type parameter
        self.inner.downcast_mut().unwrap()
    }
}

// --- These downcasts are not guaranteed to work: ---

impl<E> Report<E> {
    pub fn downcast_ref<T>(&self) -> Option<&T>
    where
        T: Display + Debug + Send + Sync + 'static,
    {
        self.inner.downcast_ref()
    }

    pub fn downcast_mut<T>(&mut self) -> Option<&mut T>
    where
        T: Display + Debug + Send + Sync + 'static,
    {
        self.inner.downcast_mut()
    }
}

// --- Some convenience methods for working with Result: ---

pub trait Context<T, E> {
    fn context<D>(self, msg: D) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static;

    fn with_context<D, F>(self, f: F) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static,
        F: FnOnce() -> D;
}

impl<T, E: miette::Diagnostic + Send + Sync + 'static> Context<T, E> for Result<T, E> {
    fn context<D>(self, msg: D) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static,
    {
        self.map_err(|e| Report::new(e).context(msg))
    }

    fn with_context<D, F>(self, f: F) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static,
        F: FnOnce() -> D,
    {
        self.map_err(|e| Report::new(e).context(f()))
    }
}

impl<T, E> Context<T, E> for Result<T, Report<E>> {
    fn context<D>(self, msg: D) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static,
    {
        self.map_err(|e| e.context(msg))
    }

    fn with_context<D, F>(self, f: F) -> Result<T, Report<E>>
    where
        D: Display + Send + Sync + 'static,
        F: FnOnce() -> D,
    {
        self.map_err(|e| e.context(f()))
    }
}
@Porges
Copy link
Contributor Author

Porges commented Aug 1, 2024

I’ve just realized this is essentially what error-stack is aiming for.

@Porges
Copy link
Contributor Author

Porges commented Aug 2, 2024

I’ve mostly worked around this in a way that works nicely by not using Report in the result types at all. This means:

  1. never using context, instead use good errors in the first place
  2. don’t attach source code to the errors, but do attach spans

Point 2 is actually much nicer since in general if you you're parsing something you’re only getting a &str and to incorporate the source code into the error result you need to clone the whole thing (which might be huge). It has actually made my code a lot cleaner to return only errors with SourceSpans inside. This gives the caller the opportunity to create a Report if they want it, so they can do something like:

Report::new(err).with_source_code(NamedSource::new(filename, source))

This has the benefit of not polluting the parsing API with filenames which may or may not be present, and the caller has more control over the ownership of the source so they can move it into the report rather than copying it.

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

1 participant