nix-community / lorri

Your project’s nix-env [maintainer=@Profpatsch,@nyarly]
Apache License 2.0
663 stars 24 forks source link

RFC: use failure crate for errors #12

Closed Profpatsch closed 3 years ago

Profpatsch commented 3 years ago

Our current code just uses a primitive “enum error wrapping approach”, which boils down to defining an Error enum type on every level of abstraction, which mostly just wraps the underlying errors that could happen. Then, on some top-level, it dies with a status ≠ 0 and prints the Debug of the outer error.

This has multiple problems in practice:

We kind of tried to keep errors from users so far by using human-panic, which we can continue using, but the error reports generated were generally of low value and did not help debugging the problem or even reproducing it.

There is multiple approaches to solving this in the rust ecosystem:

  1. The std::error::Error trait
  2. anyhow + eyre
  3. failure 4 Probably more that I didn’t see

First, anyhow and its fork eyre both look like promising approaches which will use the new restructuring of the std::errortrait to provide backtraces and make it possible to annotate errors. However, the std::error trait is in the process of being refactored, so all features, especially backtraces, are only available on nightly.

failures is the experimental crate which originally lead to the redesign of std::error, and it even goes a bit further than the rather conservative changes std is introducing. Its biggest con is that it’s not compatible with the rest of std, so if we provided a library we’d want to use the standard error trait. However, since lorri is an executable we are not restricted by this. Its biggest pro is that it provides extremely detailed usage instructions, complete with usage patterns that the library enables.

Given that failures has the best story, and we don’t have to care about being downward-compatible with the rest of the ecosystem as a library, I’d propose we go with failures and start converting the existing errors to implement its custom fail type pattern, and annotate any kind of IO error with some more information.

theduke commented 3 years ago

Hey there. I just stumbled over this by accident.

failure is deprecated and unmaintained. See the README.

It should not be used anymore for new projects.

I personally recommend anyhow, it's the most popular crate right now.

Profpatsch commented 3 years ago

As I wrote above, we are not yet on a rustc version where the error trait is stabilized & very usable (1.41).

The main reason for deprecating failure is that it’s not compatible with the rest of the ecosystem if you write a library, but lorri is an executable.

I don’t see what how a trivial library like failure is anything but a statement by the maintainer that it’s not going to change in the future. The old versions are still going to stay around after all.

Profpatsch commented 3 years ago

Also I’m super taken aback by the tendency of the rust community to always prefer the new & shiny over the stable & settled.

I personally recommend anyhow, it's the most popular crate right now.

I don’t care if something is popular, all I care about is whether it works, pulls in little transitive dependencies & won’t change its interface too much in the future.

Profpatsch commented 3 years ago

Okay, after some experiments with failure, I decided that it was just not usable enough for our use-case. The base idea is sound, but the Display and Debug for its errors is severely lacking in user-friendliness.

Instead I followed the advice above and checked out anyhow, which mostly scratches that itch.

Profpatsch commented 3 years ago

I think we can close, as sooln as #22 is merged we have a working setup, then we just need to convert all errors piece by piece as we touch them in refactors.

theduke commented 3 years ago

A little addendum: failure is deprecated not because it's not new and shiny anymore, but because it's design is suboptimal and it ends up awkward to use in practice due to several reasons, which is why the ecosystem has moved on to other libraries, and why no one wants to maintain it anymore.