rust-terminfo icon indicating copy to clipboard operation
rust-terminfo copied to clipboard

Implement strongly-typed error handling

Open SabrinaJewson opened this issue 2 years ago • 0 comments

There were a couple problems with the way the library previously did error handling:

  • Error chains didn’t inform much, as they didn’t give any context to the error
  • The library used the same error type for database loading and expansion, despite their errors beïng entirely disjoint
  • The error Display implementations didn’t follow convention

This new error API is designed with good error chains and strongly-typed errors in mind.

Examples

Examples as returned by Database::from_env():

Error: failed to load terminfo database
Caused by: no `$TERM` environment variable

Error: failed to load terminfo database
Caused by: no terminfo database found for `alacritty`

Error: failed to load terminfo database
Caused by:
   0: failed to load terminfo database /usr/share/terminfo/a/alacritty
   1: permission denied


Error: failed to load terminfo database
Caused by:
   0: failed to load terminfo database /usr/share/terminfo/a/alacritty
   1: failed to parse terminfo database

Examples as returned by expand:

Error: failed to expand terminfo string
Caused by: expansion string is invalid

Error: failed to expand terminfo string
Caused by: type mismatch

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 or terminfo::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 no source() 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 like root_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 did impl From<ParseError> for expand::Error. This was intentional under the theory that while the relationship with ParseError and expand::Error is trivial in the sense that “parsing” could only mean one thing in this context, the relationship between io::Error and expand::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.

SabrinaJewson avatar Apr 01 '23 19:04 SabrinaJewson