[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Terminal component installs the global eyre handler based on color choice #589

Open
mzabaluev opened this issue Nov 19, 2021 · 2 comments
Open

Terminal component installs the global eyre handler based on color choice #589

mzabaluev opened this issue Nov 19, 2021 · 2 comments

Comments

@mzabaluev
Copy link

The Terminal component of the framework performs initialization of the global eyre handler with color_eyre::install, unless the application's color choice is set to Never. If the handler installation fails, a panic occurs.

This is problematic, because the application should be in control of setting such things as program-global error/panic handlers.
Or, if it's really decided that the framework is so opinionated that it makes eyre setup part of its functionality, it should provide an API to override it. The colorization choice alone is a poor way to control this, because the application may want to set up error reporting differently based on other settings (e.g. whether backtraces are enabled).
mzabaluev added a commit to informalsystems/hermes that referenced this issue Nov 19, 2021
@mzabaluev
cli: Remove direct initialization of eyre handler
9b75c6d 
As long as Abscissa framework's Terminal component performs
eyre initialization under the hood and panics if it's already initialized,
the application should not do it. Unfortunately, Abscissa's initialization
of the error handler is directed only by terminal colorization choice,
which should be orthogonal to the user's preference for backtraces
in the reports.

Filed iqlusioninc/abscissa#589 to bring this
problem up to Abscissa's developers.
@tony-iqlusion
Copy link
Member

Agreed this is a problem. In general the quality of the color-eyre integration is not great at present.

It would be nice to expose something more configurable. zebrad is something I've been looking at for a reference:

https://github.com/ZcashFoundation/zebra/blob/7457edcb86ae24846f55ee1efb4b84f6ddb138e0/zebrad/src/application.rs#L261-L304

cc @yaahc

mzabaluev added a commit to informalsystems/hermes that referenced this issue Nov 22, 2021
@mzabaluev
cli: Remove direct initialization of eyre handler
2e35674 
As long as Abscissa framework's Terminal component performs
eyre initialization under the hood and panics if it's already initialized,
the application should not do it. Unfortunately, Abscissa's initialization
of the error handler is directed only by terminal colorization choice,
which should be orthogonal to the user's preference for backtraces
in the reports.

Filed iqlusioninc/abscissa#589 to bring this
problem up to Abscissa's developers.
@mzabaluev mzabaluev mentioned this issue Nov 22, 2021
Merged
5 tasks
@yaahc
Copy link
yaahc commented Nov 30, 2021

I'd probably go about this by making sure users of abscissa have a chance to install their own Handler before abcissa does automatically, and make that auto installation fail gracefully. It seems like right now you may already have the first element and only need to remove the panic to enable this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mzabaluev @yaahc @tony-iqlusion