feat(color-eyre): Add pre-hook callbacks

[joshka]

This adds the ability to add custom pre-hook callbacks to the panic / error hook. These callbacks will be called before the panic / error hook is called and can be used to print additional information or perform other actions before the panic / error hook is called (such as clearing the terminal, printing a message, etc.)

Often in an interactive TUI application, the terminal state is set to raw mode (where newlines do not automatically cause the cursor to move to the start of the next line), and is in the alternate screen buffer (which is a separate screen buffer that is used for full-screen apps).

This means that when a panic occurs, the terminal will display the panic message all janky. By adding a pre-hook callback that restores the terminal state to normal mode, the panic message can be displayed properly.

HookBuilder::default()
    .add_pre_hook_callback(Box::new(|| eprintln!("pre-hook callback")))
    .install()?

In a crossterm based app, that might look like the following instead of the more lengthy code in https://ratatui.rs/recipes/apps/color-eyre/

use crossterm::terminal::{disable_raw_mode, LeaveAlternateScreen};

HookBuilder::default()
    .add_pre_hook_callback(Box::new(|| {
        let _ = disable_raw_mode();
        let _ = execute!(stdout(), LeaveAlternateScreen);
    }))
    .install()?

[joshka]

Discussion moved to https://github.com/eyre-rs/eyre/issues/184

[joshka]

ping

[ten3roberts]

ping

Thank you

I think it is a good addition, and I've asked the other maintainers on discord if they think it is a good idea, have yet to get a response.

@yaahc, would you want this in the library?

[joshka]

Some questions to think about:

• should this be able to attach a callback for each hook type (panic / error)?
• should this be another method similar to the install method (e.g. color_eyre::install_with_pre_hook_callback(...) instead of having to resort to using the hookbuilder?

[joshka]

It turns out that my mental model for how the eyre_hook worked was a bit wrong (as noted in https://github.com/ratatui-org/ratatui/issues/1277). I thought it acted similarly to the panic hook (catch unhandled errors at the main program level and run some code then). Instead, this triggers when running the display / debug methods on Report, and it's the Termination implementation of Result that handles the actual writing to the stderr. As such maybe this is the wrong thing to do at this level (at least for the eyre hook).

The use case mentioned in https://github.com/ratatui-org/ratatui/issues/1277 was that the user wanted to have recoverable color-eyre reports that are presented to the user. These shouldn't run the pre-hook that would have been setup here. This means the code that I suggested would be inserted in the pre-hook makes more sense to put in the main function.

That leaves the panic hook. There's another approach to handling that hook that is to just replace the panic hook with one that calls the pre-hook first. This is probably a better approach. E.g.:

color_eyre::install()?;
let hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |pi| { pre_hook(); hook(pi); });

I'm inclined to close this on that basis.

[ten3roberts]

It turns out that my mental model for how the eyre_hook worked was a bit wrong (as noted in ratatui-org/ratatui#1277). I thought it acted similarly to the panic hook (catch unhandled errors at the main program level and run some code then). Instead, this triggers when running the display / debug methods on Report, and it's the Termination implementation of Result that handles the actual writing to the stderr. As such maybe this is the wrong thing to do at this level (at least for the eyre hook).

The use case mentioned in ratatui-org/ratatui#1277 was that the user wanted to have recoverable color-eyre reports that are presented to the user. These shouldn't run the pre-hook that would have been setup here. This means the code that I suggested would be inserted in the pre-hook makes more sense to put in the main function.

That leaves the panic hook. There's another approach to handling that hook that is to just replace the panic hook with one that calls the pre-hook first. This is probably a better approach. E.g.:

color_eyre::install()?;
let hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |pi| { pre_hook(); hook(pi); });

I'm inclined to close this on that basis.

I see, so your solution would be to just bubble the error all the way, and log it or in a similar vain handle it

[joshka]

I see, so your solution would be to just bubble the error all the way, and log it or in a similar vain handle it

Yes basically.