Failure crate: "soft ecosystem deprecated"

[liamsi]

• Related discussion: Light: more data in errors #149 (comment)

• Reasoning in abscissa repo: crates: Replacements for failure? iqlusioninc/abscissa#144

• Good read: https://blog.yoshuawuyts.com/error-handling-survey/

Currently:
https://github.com/interchainio/tendermint-rs/blob/f9287e04ee84e6659ea209529a1bb700ab53e45b/tendermint/Cargo.toml#L36

Used in:

• tendermint-rs/error.rs
• tendermint-rs/amino_types/validate.rs
• tendermint-rs/rpc/error.rs

We should switch to a combination of thiserror + anonmaly for the whole code base as done in #151 and #149 for the light client module.

[liamsi]

I think we should use thiserror to replace the dependency on the failure derives. To have a single error-type for the light client lib (which methods can return instead of an enum variant) anomaly looks like a very minimalistic, very good choice.

We could, of course, write a bunch of boilerplate code ourself and not depend on any other lib but this will eat our time for no good reason.

ref related issue: #139.
https://github.com/dtolnay/thiserror
https://docs.rs/anomaly/

[xla]

I think we should use thiserror to replace the dependency on the failure derives. To have a single error-type for the light client lib (which methods can return instead of an enum variant) anomaly looks like a very minimalistic, very good choice.

What does that mean for the error type itself in the light module, will it be modeled like in the anomaly example and carry an enum kind?

We could, of course, write a bunch of boilerplate code ourself and not depend on any other lib but this will eat our time for no good reason.

What is the typical boilerplate code we would have to maintain? I think it's important to know the cost/downsize.

ref related issue: #139.
https://github.com/dtolnay/thiserror
https://docs.rs/anomaly/

[liamsi]

What does that mean for the error type itself in the light module, will it be modeled like in the anomaly example and carry an enum kind?

We have an enum with different errors (ErrorKind) anyways because we want to differentiate between different errors.

What is the typical boilerplate code we would have to maintain? I think it's important to know the cost/downsize.

See for example: https://doc.rust-lang.org/src/std/io/error.rs.html#91-174 (the whole file actually)

[liamsi]

For an example using thiserror only see: https://github.com/interchainio/tendermint-rs/blob/e181405c9786becec3ad0f2e86d50466eae08674/tendermint/src/lite/errors.rs
or #149 in its current state (going change in a bit but won't force push).

[liamsi]

With issue #139 and PR #149 we introduced using thiserror and anomaly.
The main motivation was, as Tony mentioned in #149 (comment), the failure crate is kinda deprecated.

On the other hand, in #149, the combination of thiserror+anomaly proved to be useful, pragmatic and straightforward to use. In the light of this changes made to the error handling for the light client, it's worth looking into error handling in the whole repo. Not sure this worth writing up a whole ADR but it's definitely good to document the patterns used. This comment tries to serve exactly this purpose.

PR #158 makes error handling consistent across modules in this crate. The pattern used can be briefly described as follows:
All error modules now use the combination of thiserror and anomaly: every error module has a type Error which simply is a type alias to anomaly::BoxError ^[1]. Additionally, to the Error each module has different Kind of errors.
The caller can use the different Kinds to properly handle the different errors as they please and for managing control flow. See here for an example on how to get the Kind out of an Error and where this is used for control flow purposes. If another error is wrapped, the wrapped error is accessible via source. See here how this looks like in practice.

Where it makes sense, e.g., to improve readability, we use the fail!, format_err!,
or ensure! macros. Like macros in general they just save writing some same or similar lines of code over and over.

Additional reading:

• excellent discussion on similar topic: crates: Replacements for failure? iqlusioninc/abscissa#144
• good overview on error handling crates: https://blog.yoshuawuyts.com/error-handling-survey/

Note that previously, the code was structured in a very similar fashion as described above. But with the naming slightly less consistent (e.g. ValidationErrorKind) and sometimes with boilerplate code that can be removed by using the crates as described.

^[1]: Actually, there is one exception: The RPC errors need to be encoded in a particular way (see the go-code and its counterpart in rust).
Although anomaly provides serde / JSON encoding for errors, the format is slightly different. Hence, we kept the error type together with a bunch of methods which also would be provided by anomaly.