Improve error handling

[vmx]

The current error handling provides poor reporting. For example any error within Multihash or Multibase just lead to an Error::ParsingError without much further information.

I propose using https://crates.io/crates/thiserror, so that we can nest the actual underlying error.

[dvc94ch]

The best is actually to just use anyhow, see withoutboats blog. In any case, why did you remove derive(Clone) from error for no reason?

[vmx]

why did you remove derive(Clone) from error for no reason?

The problem is the std::io:error which doesn't implement Clone (https://github.com/multiformats/rust-cid/commit/eeee0de2a0326469dc9c29eb65bea5537e87d7c7).

[dvc94ch]

having a struct for each error and derive(Error) using thiserror and have all return types be of anyhow::Error works very well without penalizing the happy path or having to create an error enum for every function, because if you just have a single error type with 5 variants for example, some functions will only return as subset of errors.

[dignifiedquire]

The recommendation from withoutboats is about applications, not about libraries:

For applications, I recommend using a trait object, rather than an enum, to manage your many kinds of errors. I believe that usually trying to enumerate all of the errors every library you use can throw is more trouble than its worth, and I still assert that for most applications this will lead to higher performance by keeping the happy path less pessimized. I recommend anyhow::Error as a strictly better trait object than Box<dyn Error + Send + Sync + 'static>, not to mention one that is much easier to deal with.

From https://boats.gitlab.io/blog/post/failure-to-fehler/

I would strongly suggest not depending on anyhow in a core library like this.

Unfortunately thiserror is still missing support for nostd and is somewhat stuck it seems (ref https://github.com/dtolnay/thiserror/pull/64) so I would recommend to go the manual route as we currently do in rustcrypto land, for example: https://github.com/RustCrypto/RSA/blob/master/src/errors.rs

[dvc94ch]

I believe that I've seen him say otherwise too, although I'm having difficulty finding a quote. So there are two types of errors, errors that need to be handled by the user and errors that are fatal. In most cases if an error happens it is propagated all the way to main after which it is displayed. One way to make sure that errors that need to be handled aren't accidentally propagated is using a nested Result<Result<T, CustomError>, anyhow::Error>. Can you elaborate on the advantage of having explicit error enums?

[dvc94ch]

Note that often things that need to be handled by the user use an Option<T> or something else instead of an error in rust.

[dvc94ch]

Another problem is I actually do want to handle an error, for example a io error of some sort. A few layers up, it is really hard to get the io error:

match error {
      libipld::Error::Cid(cid::Error::Multihash(multihash::Errror::Io(io))) if io.kind() == ErrorKind::WouldBlock => {}
      libipld::Error::Multihash(multihash::Error::Io(io))) if io.kind() ... => {}
      libipld::Error::Io(io) if ... => {}
      libipld::Error::Cbor(cbor::Error::Io(io)) if ... => {}
}

This is a nuissance. With the method I proposed you can handle it easily:

if let Some(error) = error.downcast_ref(io::Error) {
    ...
}

[dvc94ch]

Put a different way, what ways are there to handle an error?

• log and continue
• log and abort
• retry
• you have multiple cases to try, for example in a parser: do a else b else c

Now in all the rust code you've written, how many errors where of the case 3 or 4? 3 is most of the time a WouldBlock or Interrupted or maybe an atomic compare and swap that failed or a transient network error. 4 is most of the time part of a recursive decent parser. 3/4 tend to get handled immediately by a library, not requiring user intervention. If you have other error handling usecases I'd love to hear them.