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

GEN-159: Local install_debug_hook #4524

Open
tisonkun opened this issue May 27, 2024 · 5 comments
Open

GEN-159: Local install_debug_hook #4524

tisonkun opened this issue May 27, 2024 · 5 comments
Assignees
Labels
area/libs > error-stack Affects the `error-stack` crate (library) area/libs Relates to first-party libraries/crates/packages (area) category/enhancement New feature or request lang/rust Pull requests that update Rust code

Comments

@tisonkun
Copy link

Related Problem

Report::install_debug_hook / Report::set_color_mode / Report::set_charset seems all global settings.

I'd prefer to switch the manner locally. For example, I'd switch the manner when returning HTTP error response to trim the Backtrace with Report::install_debug_hook::<std::backtrace::Backtrace>(|_, _| {}); as well as use ASCII and no color, but I'm OK with these settings in all other situations.

Proposed Solution

No idea. I'm not quite sure about the philosophy.

Alternatives

No response

Additional context

No response

@tisonkun tisonkun added area/libs Relates to first-party libraries/crates/packages (area) area/libs > error-stack Affects the `error-stack` crate (library) category/enhancement New feature or request lang/rust Pull requests that update Rust code labels May 27, 2024
@vilkinsons vilkinsons changed the title Local install_debug_hook H-2790: Local install_debug_hook May 29, 2024
@vilkinsons vilkinsons changed the title H-2790: Local install_debug_hook GEN-159: Local install_debug_hook May 29, 2024
@vilkinsons vilkinsons changed the title GEN-159: Local install_debug_hook GEN-159: Local install_debug_hook Jun 3, 2024
@TimDiekmann
Copy link
Member

Hi @tisonkun.

Yes, the mentioned functions are global. Do you have an idea how we could achieve the behavior? If I understand correctly you want to attach more information to the Report how this should be formatted?

Maybe, it's possible to add a function which internally adds formatting hooks to one report and use the global functions as fallback, what do you think @indietyp?

@indietyp
Copy link
Member

indietyp commented Jun 8, 2024

That should be possible. Yes. There might be some hurdles in implementing this in a way that isn't overcomplicating things, but in theory: Yes.

@tisonkun
Copy link
Author

tisonkun commented Jun 8, 2024

Do you have an idea how we could achieve the behavior

I'm not quite familiar with the codebase and don't take a closer look yet.

But here are two major considerations:

  1. It's generally possible to use a fallback strategy for preferring a local debug hook while falling back to the global one when the local one is None.
  2. The reason we need such a global setting, IIUC, is that {:?} hard code to call Debug::fmt and we don't have a place to pass the hook config. However, in my situation or anyone who wants to alter the manner locally, I'm OK to call a different method, like Report::debug_fmt(&self, debug_hook_config) to render the string.

Combine 1 and 2 may be a way to the solution. Hopefully this helps :D

@tisonkun
Copy link
Author

tisonkun commented Jul 7, 2024

After a closer look, I can see that here come two different issues:

The color mode and charset can be passed manually, by creating a Report::debug_fmt(&self, debug_hook_config) and delegate the current debug impl to it. When we'd like a local config, use the new debug_fmt instead.

The hook and context can be much more complex. But basically, print Backtrace is too large, so if we have a feature flag to turn it off, that should be enough. I think people would always want location, but not backtrace.

That said, we generally respect user-installed hooks, but the current always install backtrace built-in hook too noisy in some case.

cc @TimDiekmann @indietyp

@TimDiekmann
Copy link
Member

@indietyp and I discussed how this could be implemented and we currently figuring out what the best API would be. Most likely it will be possible to simply .attach() some debug hook. On the nightly channel, it will probably also be possible to use Error::provide to inject debug hooks. That way, it is possible for a specific error (e.g. a user-facing API endpoint error) to provide some custom debug hooks like disabling the Backtrace.

But basically, print Backtrace is too large, so if we have a feature flag to turn it off, that should be enough.

Having a dedicated feature flag for Backtraces was requested in #3353 and was implemented in #4685.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/libs > error-stack Affects the `error-stack` crate (library) area/libs Relates to first-party libraries/crates/packages (area) category/enhancement New feature or request lang/rust Pull requests that update Rust code
Development

No branches or pull requests

3 participants