color-backtrace icon indicating copy to clipboard operation
color-backtrace copied to clipboard

Support for std::backtrace::Backtrace

Open ArekPiekarz opened this issue 5 years ago • 6 comments

std::error::Error has an experimental support for providing a backtrace of type std::backtrace::Backtrace. However color-backtrace accepts only types backtrace::Backtrace and failure::Backtrace in its printing functions.

Would it be possible to add support for the std variant as well? I'm not sure how hard it would be, because it doesn't expose its inner frames.

ArekPiekarz avatar Nov 24 '19 12:11 ArekPiekarz

I don't know if the Display impl of std::backtrace::Backtrace has a stable format, so parsing it may break in the future. The Debug impl definitely has no stable format, as no Debug impls in libstd are stable.

bjorn3 avatar Nov 24 '19 13:11 bjorn3

Thanks for the issue -- I didn't realize this got merged & this is great news! I'd definitely love to support this.

It appears to me like we will have to opt for the same ugly hack as with failure backtraces, that is pasting the underlying data structs from libstd and then transmute the opaque std struct pointer into our pasted version or opt for parsing the text backtrace, as implicitly suggested by @bjorn3.

The RFC explicitly describes this minimal interface without access to the backtrace frames, so changing this would likely require another RFC.

athre0z avatar Nov 25 '19 01:11 athre0z

I had a look into this today and, unfortunately, this is not as easy to support as with failure. The std::backtrace::Backtrace impl doesn't use the vanilla backtrace_rs::BacktraceFrame structs but instead reimplements many aspects of it. We'd have to rework color-backtrace to no longer just use the backtrace_rs data-structures and build some kind of abstraction over the different representations from backtrace_rs vs std::backtrace::Backtrace.

athre0z avatar Feb 26 '20 19:02 athre0z

Alright I did the thing, I wrote a deserializer for std::backtrace::Backtrace which should hopefully be enough to implement this

https://crates.io/crates/btparse

Once I've finished with the fmt stuff I'll open a PR resolving this issue assuming someone else doesn't get to it first. The plan is to add the support as an experimental feature which enables the dependency on btparse and conditionally compiles the code using it. It should be stable API wise, and returns a Result<btparse::Backtrace, btparse::Error> so that if the format changes the backtrace parsing should just return errors.

yaahc avatar Apr 30 '20 03:04 yaahc

@yaahc You are parsing the debug format of std::backtrace::Backtrace, and the Debug formats in stdlib are specifically not guaranteed to be stable. So I don't think your solution is workable.

winstonewert avatar May 01 '20 14:05 winstonewert

@winstonewert my proposal is that the support via debug is put behind a feature flag saying unstable-stdbt or something that makes it clear that color-backtrace is providing best effort but not guarunteed parsing. The plan is to remove the feature flagged support once they add a stable iterator API to std backtrace.

Also, the btparse deserialize function returns an error if it fails to parse it, so this shouldn't even cause runtime issues, color-backtrace can just fall back to the std provided uncolored Display if it fails to parse the debug format.

I don't know about you, but I don't particularly care if my programs on nightly "break" and stop showing colored backtrace and instead show the default uncolored one, its still better than the current state which is permanently uncolored and unfiltered.

yaahc avatar May 01 '20 16:05 yaahc