4 ms·

The fact that failure conditions from both `File::open` and `read_to_string` become an `IoError` is a significant roadblock to making these errors useful. The m

by nagisa 5y ago

The fact that failure conditions from both `File::open` and `read_to_string` become an `IoError` is a significant roadblock to making these errors useful. The mechanism as described in this blog post also fails to introduce contextual information about the reason a failure has occurred. This means that errors, if implemented as described in this post, either formatted or handled don't give sufficient information to the caller/user on how to deal with the error. EDIT: I strongly recommend to use `op.map_err(|e| SomeError::Open(e, filename))?` and `op.map_err(SomeError::Read)?` as an alternative when propagating the errors. It is more typing at the location of propagation than just `?`, but the errors this approach produces are immediately actionable regardless of whether they are printed to the user or handled by a caller. Provided, of course, this pattern is applied consistently.

matklad 5y ago
Heh, I opened the comment section to link https://kazlauskas.me/entries/errors.html https://kazlauskas.me/entries/errors.html and https://sled.rs/errors.html https://sled.rs/errors.html :-)
Macha 5y ago
You can match on std::io::ErrorKind, which is available by calling kind() on your error. This effectively has the same result as error inheritance hierarchies in Java and C#. I don't think you really would treat a failure in File::open and read_to_string() differently in most applications. If you have a function doing file IO you might retry reading a read_to_string() error, but you know that the file exists because you know which function you just called. If you're at higher level of abstraction, where something is calling both open() and read_to_string() for you, are you really going to handle it differently that frequently whether it's a missing file or a file on an unavailable storage medium?
- nagisa 5y ago
  In my personal experience distinct handling of errors like these does indeed come up significantly more rarely. What ends up mattering to me much more often is how these errors are presented to the user, or what ends up in the logs in production. In those cases something like error: failed to read contents of `foo` caused by: a physical I/O error has occurred (...) and error: failed to open `foo` caused by: permission denied (...) are significantly more actionable than just the underlying I/O errors by themselves. To the point where a person reading these messages has a shot at addressing the problem without any prior knowledge of the code base. In `anyhow` adding such context via its `context` family of methods seems fairly well accepted. I hope that my comment demonstrates there's no reason why `enum` based approach would need to be any worse than what `anyhow` can achieve.
  - Macha 5y ago
    For your logs you're going to print the error's default message anyway, so you should have the underlying message of the std::io::Error as long as it's listed in the source of your wrapped error.
    - shepmaster 5y ago
      > you should have the underlying message of the std::io::Error This is a point of debate[1] among the error-handling working group. [1]: https://github.com/rust-lang/project-error-handling/issues/44 https://github.com/rust-lang/project-error-handling/issues/4...
  - shepmaster 5y ago
    > how these errors are presented to the user, or what ends up in the logs in production. In those cases something like [...] are significantly more actionable than just the underlying I/O errors by themselves. To the point **where a person reading these messages has a shot at addressing the problem** without any prior knowledge of the code base. (emphasis mine) This is a huge belief of mine as well. I call it a "semantic stacktrace". I actually go as far as to say that _most of the time_ backtraces in errors are an antipattern, as it gives a false sense that an error is actionable ("oh, just look in the code!"). I even try to make it such that each leaf error is constructed in exactly one place so that grepping for the error identifies that one location.
shepmaster 5y ago
Check out my sibling comment about SNAFU, which addresses your concerns, if I understand them.

by nagisa 5y ago

matklad 5y ago

Heh, I opened the comment section to link https://kazlauskas.me/entries/errors.html https://kazlauskas.me/entries/errors.html and https://sled.rs/errors.html https://sled.rs/errors.html :-)

Macha 5y ago

You can match on std::io::ErrorKind, which is available by calling kind() on your error. This effectively has the same result as error inheritance hierarchies in Java and C#. I don't think you really would treat a failure in File::open and read_to_string() differently in most applications. If you have a function doing file IO you might retry reading a read_to_string() error, but you know that the file exists because you know which function you just called. If you're at higher level of abstraction, where something is calling both open() and read_to_string() for you, are you really going to handle it differently that frequently whether it's a missing file or a file on an unavailable storage medium?

nagisa 5y ago
In my personal experience distinct handling of errors like these does indeed come up significantly more rarely. What ends up mattering to me much more often is how these errors are presented to the user, or what ends up in the logs in production. In those cases something like error: failed to read contents of `foo` caused by: a physical I/O error has occurred (...) and error: failed to open `foo` caused by: permission denied (...) are significantly more actionable than just the underlying I/O errors by themselves. To the point where a person reading these messages has a shot at addressing the problem without any prior knowledge of the code base. In `anyhow` adding such context via its `context` family of methods seems fairly well accepted. I hope that my comment demonstrates there's no reason why `enum` based approach would need to be any worse than what `anyhow` can achieve.
- Macha 5y ago
  For your logs you're going to print the error's default message anyway, so you should have the underlying message of the std::io::Error as long as it's listed in the source of your wrapped error.
  - shepmaster 5y ago
    > you should have the underlying message of the std::io::Error This is a point of debate[1] among the error-handling working group. [1]: https://github.com/rust-lang/project-error-handling/issues/44 https://github.com/rust-lang/project-error-handling/issues/4...
- shepmaster 5y ago
  > how these errors are presented to the user, or what ends up in the logs in production. In those cases something like [...] are significantly more actionable than just the underlying I/O errors by themselves. To the point **where a person reading these messages has a shot at addressing the problem** without any prior knowledge of the code base. (emphasis mine) This is a huge belief of mine as well. I call it a "semantic stacktrace". I actually go as far as to say that _most of the time_ backtraces in errors are an antipattern, as it gives a false sense that an error is actionable ("oh, just look in the code!"). I even try to make it such that each leaf error is constructed in exactly one place so that grepping for the error identifies that one location.

shepmaster 5y ago

Check out my sibling comment about SNAFU, which addresses your concerns, if I understand them.