Tracking Issue for panic_backtrace_config

[Mark-Simulacrum]

Feature gate: #![feature(panic_backtrace_config)]

This is a tracking issue for configuring the capture and display of backtraces in the default panic hook, as well as exposing that configuration to third-party libraries for usage.

Public API

mod panic {
    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
    #[non_exhaustive]
    pub enum BacktraceStyle {
        Short,
        Full,
        Off,
    }
    fn set_backtrace_style(BacktraceStyle);
    fn get_backtrace_style() -> Option<BacktraceStyle>;
}

This API is intended to form part of the strategy for addressing the unsoundness of the std::env::set_var API on some platforms (mostly non-Windows). See #92431 (comment) for a summary.

Steps / History

• Implementation: Support configuring whether to capture backtraces at runtime #93101
• Final comment period (FCP)
• Stabilization PR

Unresolved Questions

• Do we need to move to a thread-local or otherwise more customizable strategy for whether to capture backtraces? See this comment for some potential use cases for this.
  • Proposed answer: no, leave this for third-party hooks.
• Bikeshed on naming of all the options, as usual.
• Should BacktraceStyle be moved into std::backtrace?
  • It's already somewhat annoying to import and/or re-type the std::panic:: prefix necessary to use these APIs, probably adding a second module to the mix isn't worth it.
• Should get_backtrace_style be exposed?
  • The Option<BacktraceStyle> return type looks a little weird, and may not mean the intuitive thing -- None represents Unsupported, not "not set" or "don't print backtraces", as might be initially assumed.
  • We may want a std::panic::report_backtrace(&mut dyn Write) instead which internally knows how to format things.

[yaahc]

• It's already somewhat annoying to import and/or re-type the std::panic:: prefix necessary to use these APIs, probably adding a second module to the mix isn't worth it.

I think it depends on how we expose the Short and Full format displays to users. If we expect users to use fmt flags to configure the format when printing std::backtrace::Backtrace then we'd probably want to associate the style with the panic module.

I do worry though about how this style of API will look like in practice, I expect you'd probably have to write something like this every time you want to copy the std panic hook behavior:

match get_backtrace_style() {
    Short => // use correct fmt flags for short
    Full => // use correct fmt flags for full
    // ...
}

Which feels a little unfortunate. This is a strawman proposal but I would personally rather use something that can be easily expressed in one line, first idea I can come up with is this:

println!("{}", backtrace.display(get_backtrace_style()));

And if we were to provide an adapter API for associating a style with a backtrace when printing it then I imagine we'd want to include the style type in the backtrace module.

[Mark-Simulacrum]

For the trivial "just print it" case, I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better (or takes a &mut dyn Write, maybe) -- capturing and resolving the backtrace via std::backtrace::Backtrace in panic handling may want to e.g. avoid allocations and some of the other complexity associated with a more general API like std::backtrace::Backtrace.

If you're doing something more complicated that doesn't just want to adjust the surrounding material but alter the backtrace printing directly, it may need some API design if we don't wan to stabilize some fairly low-level APIs (e.g., https://docs.rs/backtrace/latest/backtrace/fn.trace.html and similar) that may not be a good fit for std itself.

I'm not sure there's a great answer here, though; I think the tradeoffs are particularly acute around this surface area and difficult to judge.

[yaahc]

For the trivial "just print it" case, I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better

agreed, I've felt like we should be exposing the allocation-lite backtrace printing interface we use in the panic hook for a while now.

If you're doing something more complicated that doesn't just want to adjust the surrounding material but alter the backtrace printing directly, it may need some API design if we don't wan to stabilize some fairly low-level APIs (e.g., https://docs.rs/backtrace/latest/backtrace/fn.trace.html and similar) that may not be a good fit for std itself.

oh interesting, I can see us going this direction, though the usage of trace seems somewhat opaque / complex so like you said we'd need some design discussion on that or some more informative examples. For now I am a big enough fan of the split you describe and think we should plan on keeping BacktraceStyle in panic and add a std::panic::report_backtrace(&mut dyn Write) and let it read the backtrace style internally.

Thinking about it though, this approach definitely reduces the value of the get_backtrace_style function, since it's not really making it easy to copy the behavior of the panic hook when printing std::backtrace::Backtraces, and you wouldn't even need to use it when working with report_backtrace.

[RalfJung]

@rust-lang/libs-api I'd like to propose to stabilize these APIs as-is. The API matches what we have done with environment variables for a long time, just a bit more type-safe. There's more that can be done here (see the posts by @yaahc above), but I don't think any of this should be a blocker -- given in particular that this feature is on the critical path for reducing the use of set_var in the ecosystem.

[m-ou-se]

set_backtrace_style() is almost the same as setting RUST_BACKTRACE, except that it doesn't enable Backtrace::capture(), right? Should it? Or is that intentional?

[RalfJung]

Oh, interesting. On the one hand it seems strange if a std::panic function affects std::backtrace behavior, on the other hand ideally this function can replace the environment variable entirely.

Maybe the enum and methods should be moved to std::backtrace?

[target-san]

I suspect a direct fn std::panic::report_backtrace() which internally prints to stderr may be better

Sorry for interfering, though I'd suggest previous Backtrace::display(...) API for one simple reason - ability to gather backtrace and then print it using builtin styles elsewhere. It's a bit inconvenient that you either use default default panic hook and get nice short stacktraces or have to emulate it by hand for std::backtrace::Backtrace.

[tbu-]

Usually, we omit get_ from getters. So it'd be called std::panic::backtrace_style and not std::panic::get_backtrace_style.