rust-lang-deprecated / failure

Error management
https://rust-lang-nursery.github.io/failure/
Apache License 2.0
1.42k stars 140 forks source link

How many error types should libraries have in their APIs? #7

Open withoutboats opened 6 years ago

withoutboats commented 6 years ago

Should libraries have 1 error type in their API, or many? error-chain today strongly encourages having a single error type, which is probably not always the best approach.

The primary reason to have multiple error types is that you can then easily restrict a function to throwing only 1 particular error type. The primary disadvantage is that it becomes harder to deal with cases where there are multiple error types.

For example, in the error-chain docs, an example is shown with four variants:

Notably, when parsing a toolchain identifier from a string, only the first two variants could occur. It would make sense, then, to have a situation like this:

#[derive(Debug, Fail)]
enum ToolchainError {
    InvalidName,
    InvalidVersion,
}

impl FromStr for ToolchainId {
     type Err = ToolchainError;
}

But this raises the question: should the larger section of code, which reads/parses a file containing toolchain ids in a toml document, have its own new error type, adding the other variants, or should it just return Error?

The questions posed then are:

steveklabnik commented 6 years ago

I'm not going to say that the stdlib is perfect, but it's pretty good. And it's one of the largest crates. If you look at it, there's a bunch of patterns:

  1. No single error type for the whole stdlib
  2. Some modules have a single error type, like std::io::Error. This is convenient, but also leads to some functions not being able to return all errors, which is not the best
  3. Some functions have an error type for them only, like https://doc.rust-lang.org/stable/std/env/enum.VarError.html
withoutboats commented 6 years ago

Some modules have a single error type, like std::io::Error. This is convenient, but also leads to some functions not being able to return all errors, which is not the best

I think io::Error is a pretty interesting example. Its necessarily a very open-ended error, since some operations will produce different errors on different platforms. And I don't think that we necessarily got it completely right!

What's sort of unfortunate is that error-chain is heavily modeled on io::Error, when I think io::Error is really a very unique case. Hard to know what guidance to give though since we've built an ecosystem that copies from io::Error.

U007D commented 6 years ago

You may be asking experience Rustaceans for guidance, but in case you are looking for newb thoughts as well, here is mine:

I think you've taken exactly the right approach with Failure not being opinionated on how many error types a crate should define. There are probably reasonable use cases for error types at all three scopes (per crate, per module and per function).

I am still in the early design phase for my main Rust project, but, by way of analogy, while io::Error explicitly discourages exhaustive matching, for safety reasons, I want my own error types to be exhaustively matched. (So error-chain's practice of adding an implicit Message error to any definition was unfortunate from my perspective). The point being, if I had a new error type to my library, I do want all my dependent modules to break, so that we carefully consider what path of execution we want to occur.

I see this as an analog to whether a given library should define only one error type or a multiple error types. In my earlier Rust projects, I used to use Box as my return type, but later realized that returning an explicit error type instead (with exhaustive matching) is safer for my use case.

This does not suggest that either policy is right or wrong, just that there are different needs out there, and I'm (very) glad Failure is not opinionated on the question of number of error types defined.

Arnavion commented 6 years ago

To clarify some things about error-chain:

@withoutboats error-chain today strongly encourages having a single error type

@U007D I think you've taken exactly the right approach with Failure not being opinionated on how many error types a crate should define. There are probably reasonable use cases for error types at all three scopes (per crate, per module and per function).

In common uses of error-chain, users do make one crate-level error type and throw everything in it. But nothing about error-chain requires there to be only one crate-level error type. Failure is not different from error-chain in this regard. Handling the combinatorial explosion of error enums when combining function-level error types (without falling back to Box<Error> or failure::Error) is a problem for both.

@U007D I want my own error types to be exhaustively matched. (So error-chain's practice of adding an implicit Message error to any definition was unfortunate from my perspective).

The Msg(String) member is optional in derive-error-chain, and there is a PR to make it optional in error-chain as well. That said, error-chain also marks the generated enum as non-exhaustive so it won't meet your requirement anyway. derive-error-chain does not have this problem.

U007D commented 6 years ago

Thank you for the clarifications, @Arnavion. I learned some things about error-chain.

shepmaster commented 6 years ago

My default suggestion is one per module. Things logically grouped together often have similar error cases.

wswartzendruber commented 6 years ago

Have any new best practices emerged on this topic?