zkat / miette

Fancy extension for std::error::Error with pretty, detailed diagnostic printing.
https://docs.rs/miette
Apache License 2.0
1.85k stars 106 forks source link

Converting a `Report` to a `Box<dyn Diagnostic>` wipes all `Diagnostic` information #369

Closed TheLostLambda closed 1 week ago

TheLostLambda commented 2 months ago

I suspect this is a super subtle issue in the crazy vtable code, but here is a relatively compact reproduction of the issue:

use miette::{miette, Diagnostic, LabeledSpan, Severity};

fn main() {
    let report = miette!(
        severity = Severity::Error,
        code = "expected::rparen",
        help = "always close your parens",
        labels = vec![LabeledSpan::at_offset(6, "here")],
        url = "https://example.com",
        "expected closing ')'"
    );
    assert_eq!(report.to_string(), "expected closing ')'".to_owned());
    assert_eq!(report.severity(), Some(Severity::Error));
    assert!(report.code().is_some());
    assert!(report.help().is_some());
    assert!(report.labels().is_some());
    assert!(report.url().is_some());

    let report_ref: &dyn Diagnostic = report.as_ref();
    assert_eq!(report_ref.to_string(), "expected closing ')'".to_owned());
    assert_eq!(report_ref.severity(), Some(Severity::Error));
    assert!(report_ref.code().is_some());
    assert!(report_ref.help().is_some());
    assert!(report_ref.labels().is_some());
    assert!(report_ref.url().is_some());

    let report_box: Box<dyn Diagnostic> = report.into();
    assert_eq!(report_box.to_string(), "expected closing ')'".to_owned());
    assert_eq!(report_box.severity(), None);
    assert!(report_box.code().is_none());
    assert!(report_box.help().is_none());
    assert!(report_box.labels().is_none());
    assert!(report_box.url().is_none());
}

The Error / Display impl is still totally okay, and all of the Diagnostic stuff that goes though the Deref impl seems to work fine — both .as_ref() and calling things like .code() on a Report directly — so the issue does not seem to be present here:

impl Deref for Report {
    type Target = dyn Diagnostic + Send + Sync + 'static;

    fn deref(&self) -> &Self::Target {
        unsafe { ErrorImpl::diagnostic(self.inner.by_ref()) }
    }
}

NOTE: I'm now moving away from the example above, to the context where this issue occurs in my code!

The issue seems to come specifically from (I believe) somehow attaching the wrong vtable? Here the data of the error stays the same — the source code information is kept — but calling .source_code() seems to just fall back on the default None impl?

impl From<Report> for Box<dyn Diagnostic + Send + Sync + 'static> {
    fn from(error: Report) -> Self {
        dbg!("impl From<Report> for Box<dyn Diagnostic + Send + Sync + 'static>");
        dbg!(&error);
        dbg!(error.source_code().is_some());
        let outer = ManuallyDrop::new(error);
        unsafe {
            // Use vtable to attach ErrorImpl<E>'s native StdError vtable for
            // the right original type E.
            let res = (vtable(outer.inner.ptr).object_boxed)(outer.inner);
            dbg!(&res);
            dbg!(res.source_code().is_some());
            res
        }
    }
}

Outputs, in my case:

[/home/tll/.cargo/git/checkouts/miette-68017d45f967249d/5f42f10/src/eyreish/error.rs:750:9] "impl From<Report> for Box<dyn Diagnostic + Send + Sync + 'static>" = "impl From<Report> for Box<dyn Diagnostic + Send + Sync + 'static>"
[/home/tll/.cargo/git/checkouts/miette-68017d45f967249d/5f42f10/src/eyreish/error.rs:751:9] &error = Error {
    source_code: NamedSource {
        name: "test",
        source: "<redacted>",
        language: None,
    ,
    errors: [
        Unexpected {
            span: Span(
                0,
                49,
            ),
            kind: "node",
            message: "unexpected node `P`",
        },
        MissingNode {
            message: "child node `elements` is required",
        },
    ],
}
[/home/tll/.cargo/git/checkouts/miette-68017d45f967249d/5f42f10/src/eyreish/error.rs:752:9] error.source_code().is_some() = true
[/home/tll/.cargo/git/checkouts/miette-68017d45f967249d/5f42f10/src/eyreish/error.rs:758:13] &res = Error {
    source_code: NamedSource {
        name: "test",
        source: "<redacted>",
        language: None,
    ,
    errors: [
        Unexpected {
            span: Span(
                0,
                49,
            ),
            kind: "node",
            message: "unexpected node `P`",
        },
        MissingNode {
            message: "child node `elements` is required",
        },
    ],
}
[/home/tll/.cargo/git/checkouts/miette-68017d45f967249d/5f42f10/src/eyreish/error.rs:759:13] res.source_code().is_some() = false

The error and res structs remain the same, but calling the .source_code() method does not!

Any pointers on where to start fixing this would be excellent! The unsafe vtable code is beyond anything I've ever done in Rust, so figuring it out is a gradual process...

TheLostLambda commented 2 months ago

Much more mundane than I thought... I've got a fix that I'll make a PR for!