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

Wrapping other Errors without losing diagnostics #172

Closed
Porges opened this issue May 13, 2022 · 6 comments
Closed

Wrapping other Errors without losing diagnostics #172

Porges opened this issue May 13, 2022 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Porges
Copy link
Contributor

Porges commented May 13, 2022

I’m having trouble figuring out how to directly (i.e. not with a set of errors like related) wrap another Miette-embellished error type without losing its diagnostic information.

I started with (for example):

#[derive(Error, Diagnostic, Debug)]
pub enum Error {
    // …
    #[error("Input file had a syntax error")]
    #[diagnostic(code(mylib::syntax_error))]
    SyntaxErr(#[from] knuffel::Error),
}

However, when printing this drops the diagnostic information from the knuffel::Error.

It seems like source/diagnostic_source are the solution to this (and from implies source), however if I use them like this:

#[derive(Error, Diagnostic, Debug)]
pub enum Error {
    // …
    #[error("Input file had a syntax error")]
    #[diagnostic(code(mylib::syntax_error))]
    SyntaxErr{
        #[from]
        #[source]
        #[diagnostic_source]
        source: knuffel::Error,
    },
}

… I get problems with the Diagnostic macro:

error[E0599]: the method `as_ref` exists for reference `&knuffel::Error`, but its trait bounds were not satisfied
  --> src/lib.rs:8:17
   |
8  | #[derive(Error, Diagnostic, Debug)]
   |                 ^^^^^^^^^^ method cannot be called on `&knuffel::Error` due to unsatisfied trait bounds
   |
  ::: …/knuffel-2.0.0/src/errors.rs:27:1
   |
27 | pub struct Error {
   | ---------------- doesn't satisfy `knuffel::Error: AsRef<_>`
   |
   = note: the following trait bounds were not satisfied:
           `knuffel::Error: AsRef<_>`
           which is required by `&knuffel::Error: AsRef<_>`
   = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0599`.

#[diagnostic_source] actually doesn’t fail if I lift it onto the enum variant (as opposed to its field) but then it doesn’t seem to have any effect.

I feel like this should be something straightforward, what am I missing? 😊

@zkat
Copy link
Owner

zkat commented May 13, 2022

/cc @matthiasbeyer looks like something might have gone sideways with diagnostic_source?

@zkat zkat added bug Something isn't working help wanted Extra attention is needed labels May 13, 2022
@TheNeikos
Copy link
Contributor

I believe this is fixed by #169 ?

@Porges could you potentially try that branch?

@Porges
Copy link
Contributor Author

Porges commented May 15, 2022

#169 seems to fix the compilation error but I still don’t get any diagnostics from the nested error.

Current code is:

#[derive(Error, Diagnostic, Debug)]
pub enum Error {
    // …
    #[error("Input file had a syntax error")]
    #[diagnostic(code(mylib::syntax_error))]
    SyntaxErr(
        #[from]
        #[diagnostic_source]
        knuffel::Error,
    ),
}

Output is:

Error: mylib::syntax_error

  × Input file had a syntax error
  ╰─▶ error parsing KDL

(Have tried the variant with a single-field struct as well, with the same result.)

Edit: Oh, is this expected behaviour? Quoting the diagnostic_source docs (my emphasis):

While this has no effect on the existing reporters, since they don’t use that information right now, APIs who might want this information will have no access to it.

@TheNeikos
Copy link
Contributor

#169 seems to fix the compilation error but I still don’t get any diagnostics from the nested error.

Indeed, this is something that is being discussed here, where I suggested a potential path forward, but a deeper discussion on where the crate should head then started: #170

KtorZ added a commit to aiken-lang/aiken that referenced this issue Jan 21, 2023
  This is a bit annoying as we are forced to use #[related] here which isn't quite what we want.
  Ideally, this would use #[diagnostic_source] but, there's a bug upstream. See: zkat/miette#172.
@azriel91
Copy link

azriel91 commented Apr 20, 2023

Heya, I'd like to revive this discussion.

I've been keeping my project compatible with miette, and the structure of how the framework is used is something like the following:

consumer --> framework_plugin (potentially many of these)
      \             \
       \             v
        '--------> framework

Each of these 3 crates provides mietee::Diagnostic error types:

Example error types:
// Framework crate
#[derive(miette::Diagnostic)]
enum FrameworkError {
    One {
        #[source_code] source: miette::NamedSource,
        #[label]       error_span: Option<miette::SourceOffset>,
    },
}

// Plugin crate, depends on framework
#[derive(miette::Diagnostic)]
enum PluginError {
    One {
        #[source_code] source: miette::NamedSource,
        #[label]       error_span: Option<miette::SourceOffset>,
    },
    Two { .. },

    FrameworkError(
        #[diagnostic_source]
        #[source]
        FrameworkError,
    ),
}

// Consumer crate, depends on Framework and Plugin crates
#[derive(miette::Diagnostic)]
enum ConsumerError {
    One {
        #[source_code] source: miette::NamedSource,
        #[label]       error_span: Option<miette::SourceOffset>,
    },

    FrameworkError(
        #[diagnostic_source]
        #[source]
        FrameworkError,
    ),

    PluginError(
        #[diagnostic_source]
        #[source]
        PluginError,
    ),
}

Currently when an error is returned, the FrameworkError and PluginError diagnostics aren't shown, unless the consumer deconstructs and descends into each diagnostic source variant manually (see main.rs, env_deploy_cmd.rs):

// error is a `ConsumerError`
let diagnostic: &dyn Diagnostic = match error {
    // Repeat this for all the plugins
    ConsumerError::PluginError(PluginError::FrameworkError(e)) => e,
    ConsumerError::PluginError(e) => e,

    ConsumerError::FrameworkError(e) => e,
    _ => error,
};

So ideally the printing of those inner diagnostics could / would be automatic, since the #[diagnostic_source] attribute is already deliberately opt-in.

Given the above, I propose the following behaviour:

  • If a struct or enum variant is a newtype with just one #[diagnostic_source] field, and optionally a PhantomData, then the rendering of the diagnostic should be transparent (like serde).
  • If a struct or enum variant is a Diagnostic, has its own #[source] and #[label]s as well as a #[diagnostic_source] field, then:
    • The struct / variant's diagnostics are rendered.
    • The field's diagnostics may also be rendered:
      • Whether members' diagnostics are rendered by default is controllable through the MietteHandlerOpts.
      • Maybe limit how deep the renderer will proceed into diagnostic_source sub-fields

Would that be the right direction for miette? If so, I'll take a look at #170 and see whether to pick up from there, or begin a new branch.

@Porges
Copy link
Contributor Author

Porges commented Jul 20, 2023

This is now solved thanks to #170. #[diagnostic_source] works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants