n0-computer / beetle

Other
34 stars 15 forks source link

remove anyhow in all lib crates #80

Open b5 opened 1 year ago

b5 commented 1 year ago

Things we need to establish a convention for what we doing about anyhow::Result return types?

I've opened n0-computer/iroh#490 to explore idioms, and in this one I'm experimenting with defining a Result alias in each module that replaces the anyhow::Result type.

Crates we can skip:

Crates that need replacing:

timing

I'm not expecting us to be able to pull this off overnight, and don't want to hurt developer velocity too much. To that end, I'm intentionally not attaching this tracking issue to a milestone. It'll be done whenever it's done.

With that said, once anyhow has been excised from a crate, let's not allow adding it back.

matthiasbeyer commented 1 year ago

Btw you can tick off iroh-car, it does not use anyhow! :wink:

matthiasbeyer commented 1 year ago

In my efforts in n0-computer/iroh#492 I noticed that all the crates are so much integrated into eachother that it is not possible to replace anyhow one-by-one. Thus, my patches would transform all crates in one go ... I guess I'll submit a completely new PR once I am done and then we will see whether we can still split it up (I'll commit as cleanly as possible) or whether a one-step-for-everything transformation is feasible :thinking:

flub commented 1 year ago

I'm not sure doing this blanket for all errors makes a lot of sense. Especially if they're being replaced with module-wide catchall error types.

To me the main motivation to switch away from anyhow to a dedicated error type is if you notice you're having to use anyhow's downcasting to handle a particular error. From a library's public API point of view that's kind of painful since every time you switch error type you make a backwards-incompatible change. However the only way to avoid this is to give (almost) each function it's dedicated error type that is future-extensible. That's also not a very practical solution.

matthiasbeyer commented 1 year ago

To me the main motivation to switch away from anyhow to a dedicated error type is if you notice you're having to use anyhow's downcasting to handle a particular error. From a library's public API point of view that's kind of painful since every time you switch error type you make a backwards-incompatible change.

This extends also to the following issue: If the library uses a Stringly typed error (bail!("Something happened")), then downcasting does not even help, you would also need to do string-matching on the user side of things... And I guess we all know that stringly typed APIs are hell :laughing:

However the only way to avoid this is to give (almost) each function it's dedicated error type that is future-extensible. That's also not a very practical solution.

I agree, partially. Some users might not bother about errors from irohs libraries in their applications. They can just .map_err(anyhow::Error::from) (or rather ?) in their code. But the users wo do care, they can catch and interact with error types much more efficiently!

Rather than:

match some_function() {
    Err(e) => {
        return Err(e.downcast_ref::<SomeError>()
            .map(MyErrorType::from)
            .or_else(|| e.downcast_ref::<SomeOtherError>().map(MyErrorType::from))
            .or_else(|| e.downcast_ref::<EvenOtherError>().map(MyErrorType::from)));
    }
    // ...
}

or even more complicated things, they can just

match some_function() {
    Err(iroh_something::Error::Ups) => handle_something_error(),
    Err(iroh_something::Error::Ooops) => handle_some_other_err(),
    // ... and so on
}