Use std::io::Error instead of own error type #2
Conversation
| |path| format!("failed to read from file `{}`", path), | ||
| ) | ||
| .map_err(Error::into_io_error) | ||
| (&(**self).file) |
There was a problem hiding this comment.
I only learned that there are trait implementations for &File today and I struggled to get this re-borrow to work. Is there a cleaner way to do this that you know?
| Ok(result) => Ok(result), | ||
| Err(error) => Err(Error::new(error, context(&self.path.display()))), | ||
| } | ||
| } |
There was a problem hiding this comment.
I replaced try_exec with just using map_err across the board
There was a problem hiding this comment.
The reason for try_exec was to let each function provide a custom error message based on the operation that failed - I would prefer not to throw that away
There was a problem hiding this comment.
I see! I've always thought of errors missing paths as the only issue with Rust's std FS errors, but they also don't include what operation they tried to perform.
Do you think it would make sense to re-introduce that custom error message by using an enum so that we can avoid allocating/formatting the error message?
There was a problem hiding this comment.
An enum would work, although since we need to clone the PathBuf already I don't think theres much advantage over just using format! directly
| #[derive(Debug)] | ||
| pub(crate) struct Error { | ||
| source: io::Error, | ||
| path: PathBuf, |
There was a problem hiding this comment.
As I mentioned in my other comment, this should have some kind of context about what operation failed
I'm also not a fan of printing the source error message in the Display impl because a generic consumer who just iterates over the error causes will end up repeating the error message like
failed to get configuration
caused by: No such file or directory (os error 2) in /my/file
caused by: No such file or directory (os error 2)
LPGhatguy
force-pushed
the
use-io-error
branch
from
7ab7784
to
c8f69eb
Compare
|
I added more specific error messages back to each error variant. I tried the enum approach, it has the nice advantage that error messages aren't duplicated and helps keep the implementation tidy, which I like. I've also fixed the source-printing issue you mentioned. I think I need to go back through some of my projects and fix the way they print errors... |
|
Thanks! |
|
I've just published version 2.0.0 with these changes |
Closes #1.
This PR converts the crate's public functions to return
std::io::Errorinstead of a custom error type. I introduced a new custom error type that's internal to the crate and then updated all interfaces to use it.This is a breaking change!