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

Support for std::backtrace::Backtrace #24

Open
ArekPiekarz opened this issue Nov 24, 2019 · 6 comments
Open

Support for std::backtrace::Backtrace #24

ArekPiekarz opened this issue Nov 24, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@ArekPiekarz
Copy link

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.

@bjorn3
Copy link

bjorn3 commented Nov 24, 2019

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.

@athre0z athre0z added the enhancement New feature or request label Nov 24, 2019
@athre0z
Copy link
Owner

athre0z commented Nov 25, 2019

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
Copy link
Owner

athre0z commented Feb 26, 2020

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.

@yaahc
Copy link
Contributor

yaahc commented Apr 30, 2020

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.

@winstonewert
Copy link

@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.

@yaahc
Copy link
Contributor

yaahc commented May 1, 2020

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants