Implement strongly-typed error handling #37
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were a couple problems with the way the library previously did error handling:
Display
implementations didn’t follow conventionThis new error API is designed with good error chains and strongly-typed errors in mind.
Examples
Examples as returned by
Database::from_env()
:Examples as returned by
expand
:Unfortunately, we do end up with significantly many more error types in our public API. I believe this to be an acceptable cost since there are inherently quite a lot of different ways operations can fail and this is how to achieve nice-looking error source chains. Additionally, they’re all cordoned off in their different modules (for example
terminfo::database::LoadError
orterminfo::expand::StackUnderflow
).Design note 1: I used quite a lot of
#[non_exhaustive]
unit structs in the implementations of these errors. The number of types overall could be reduced by combining these into enums, but unit structs for the root-cause error types, i.e. the ones with nosource()
that don’t end in*Error
, i.e.terminfo::{expand::{TypeMismatch, StackOverflow}, database::{NoTerm, NotFound}}
, have the advantage that one can downcast directly into them from functions likeroot_cause
.Design note 2: I didn’t put
#[non_exhaustive]
on most the enums. This was intentional — I guessed that it’s unlikely that new variants would need to be added in future. However I could be wrong about this.Design note 3: I didn’t
impl From<io::Error> for expand::Error
, even though I didimpl From<ParseError> for expand::Error
. This was intentional under the theory that while the relationship withParseError
andexpand::Error
is trivial in the sense that “parsing” could only mean one thing in this context, the relationship betweenio::Error
andexpand::Error
is nontrivial in the sense it’s not immediately clear what “I/O” is beïng talked about if it is done by the expansion process. As a concrete example of a different I/O operation that could occur, one thing this library does not implement (AFAICT) is millisecond delays during expansion, but I think (from reading the manpage) they’re technically sometimes required.