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

Fix #369 (Loss of `Diagnostic` information when `Report` is converted to `Box<dyn Diagnostic>`) #370

Closed TheLostLambda closed 1 week ago

TheLostLambda commented 2 months ago

Fixes #369

The culprit was:

https://github.com/zkat/miette/blob/ea4296dacec3b0e4762281d9d115c1bd69ecfac3/src/eyreish/error.rs#L728

In conjunction with:

https://github.com/zkat/miette/blob/ea4296dacec3b0e4762281d9d115c1bd69ecfac3/src/eyreish/error.rs#L531-L537

Which converts a Box<ErrorImpl<E>> into the Box<dyn Diagnostic>, hence the resultant object ends up with the empty Diagnostic impl defined above!

This fix moves the _object out of the Box<ErrorImpl<E>> to create a Box<E> before conversion to Box<dyn Diagnostic>, attaching the correct Diagnostic impl for the original error E.

Miri seems happy, and I've actually only added safe code, even though it's in an unsafe function because of .boxed() which I left untouched.


As a side note, the only reason converting to Box<dyn Error> worked is because of this manual pass-through done here: https://github.com/zkat/miette/blob/ea4296dacec3b0e4762281d9d115c1bd69ecfac3/src/eyreish/error.rs#L719-L726

But now that we aren't ever trying to convert ErrorImpl<E>s into trait objects anymore — directly converting Es instead — I've deleted that unused code.

TheLostLambda commented 1 week ago

Thanks a ton Kat! :heart_eyes:

zkat commented 1 week ago

thanks for your patience. I'll try and push out a new release soon :)

TheLostLambda commented 1 week ago

No rush at all — take your time and thanks a ton for looking after things!