Add must-install feature, so that a non-default handler can be the on…

[cr1901]

…ly handler consuming .text.

A while ago, you mentioned:

I can look into adding feature flags to eyre to disable it's own default reporter completely so that you must install a reporter or it panics, which would hopefully reduce the .text. Or maybe not, I'm kinda guessing here, this is a bit lower level than I normally go.

I implemented this feature myself out of curiosity, and it seems to have done the trick. It'll require modifying simple-eyre's Cargo.toml, but I shaved a good 8k off my .text segment with this feature :D!

[cr1901]

I needed commit 598a179 to avoid patching problems locally.

[WaffleLapkin]

What is the state of this PR?

[yaahc]

I think we just need to update the test-suite to properly handle this feature. Something like:

• Update the ci.yml config to include a test job for the new feature
• Add tests for the new feature
• Update existing tests to make them not fail when the new feature is installed (maybe with a cfg'ed install on tests to install a handler when the feature is enabled?)

Also, a nice to have but not required would be adding documentation for the new feature. It looks like we don't already have a section for this but I'd like to add one, probably based off of tracing's similar section: https://docs.rs/tracing/latest/tracing/#crate-feature-flags

[cr1901]

Incidentally, I circled back to the project using this PR yesterday on a lark. I don't think I thought through edge cases sufficiently:

Since the feature is not enabled by default, the default behavior is back-compatible with previous eyre versions. But if a dep or transitive dep uses eyre and enables the must-install feature, I think it's possible that code that previously compiled will no longer do so until they add e.g. simple_eyre::install()?; to their root (probably binary) crate. Does must-install count as a "negative" feature taking away functionality (the default handler)?

Additionally, are the eyre installers idempotent- i.e. safe to call multiple times, where only the call to install for a given handler has any effect? Can multiple eyre handlers safely exist in the same application (if more than one is brought in by deps)?

[yaahc]

Since the feature is not enabled by default, the default behavior is back-compatible with previous eyre versions. But if a dep or transitive dep uses eyre and enables the must-install feature, I think it's possible that code that previously compiled will no longer do so until they add e.g. simple_eyre::install()?; to their root (probably binary) crate. Does must-install count as a "negative" feature taking away functionality (the default handler)?

Ah, I think you're right on this one. We probably want to instead add an enabled by default auto-install feature that people can disable to get the must-install behavior, that way downstream conflict causes the non-breaking / less .text efficient behavior.

Additionally, are the eyre installers idempotent- i.e. safe to call multiple times, where only the call to install for a given handler has any effect? Can multiple eyre handlers safely exist in the same application (if more than one is brought in by deps)?

Yes, set_hook is idempotent and no, you cannot have multiple eyre handlers installed simultaneously. With the current design you'd have to find some way to compose the various handlers you'd want to use then install that as one compound handler.

[cr1901]

With the current design you'd have to find some way to compose the various handlers you'd want to use then install that as one compound handler.

Ahh I see now- Report for the various crates is an export from eyre: https://github.com/yaahc/simple-eyre/blob/master/src/lib.rs#L58

Okay, I will rework the feature into auto-install and address your checklist. Hopefully will do it tonight or tomorrow; my bandwidth has been quite weird lately.

[cr1901]

Right now, at least test_boxed fails with a panic about 'a handler must always be installed if the auto-install feature is disabled':

william@xubuntu-dtrain:~/Projects/embedded/eyre/tests$ cargo test --no-default-features

...

running 4 tests
test test_boxed_sources ... FAILED
test test_boxed_eyre ... FAILED
test test_boxed_str ... FAILED
test test_boxed_thiserror ... FAILED

failures:

---- test_boxed_sources stdout ----
thread 'test_boxed_sources' panicked at 'a handler must always be installed if the `auto-install` feature is disabled', tests/test_boxed.rs:52:25

---- test_boxed_eyre stdout ----
thread 'test_boxed_eyre' panicked at 'a handler must always be installed if the `auto-install` feature is disabled', tests/test_boxed.rs:41:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_boxed_str stdout ----
thread 'test_boxed_str' panicked at 'a handler must always be installed if the `auto-install` feature is disabled', tests/test_boxed.rs:19:25

---- test_boxed_thiserror stdout ----
thread 'test_boxed_thiserror' panicked at 'a handler must always be installed if the `auto-install` feature is disabled', /rustc/532d3cda90b8a729cd982548649d32803d265052/library/core/src/convert/mod.rs:546:9


failures:
    test_boxed_eyre
    test_boxed_sources
    test_boxed_str
    test_boxed_thiserror

I'm thinking of solving this by adding a maybe_install_handler function to tests/common, something like this:

fn maybe_install_handler() -> Result<(), InstallError> {
    if cfg!(not(feature = "auto-install")) {
        set_hook(Box::new(DefaultHandler::default_with))
    } else {
        Ok(())
    }
}

Unfortunately, this doesn't work because default_with is private:

error[E0624]: associated function `default_with` is private
   --> tests/common/mod.rs:18:52
    |
18  |         set_hook(Box::new(Box::new(DefaultHandler::default_with)))
    |                                                    ^^^^^^^^^^^^ private associated function
    |
   ::: /home/william/Projects/embedded/eyre/src/lib.rs:708:5
    |
708 |     fn default_with(error: &(dyn StdError + 'static)) -> Box<dyn EyreHandler> {
    |     ------------------------------------------------------------------------- private associated function defined here

Is there a way to create a DefaultHandler in the public API, and if not, should there be a way in light of this new feature?

[yaahc]

Is there a way to create a DefaultHandler in the public API, and if not, should there be a way in light of this new feature?

I think the answer to that is no, and yes.

[cr1901]

Hmmm, why is the workflow only showing10 expected checks? Many combinations of features and Rust compilers seem to be missing...

[yaahc]

Hmmm, why is the workflow only showing10 expected checks? Many combinations of features and Rust compilers seem to be missing...

I'm guessing that may have been because I had to approve the CI run

[cr1901]

My changes to simple_eyre are ready when eyre 0.6.8 is released. In the meantime, I'm just depending on the git repo eyre; the new auto-install feature (well, disabling it anyway!) is working well for my use case :).