-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revise error type (copy from error_details branch) #34
Conversation
I would make sure the |
ErrorKind::Transient | ErrorKind::NotReady => true, | ||
_ => false, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never know how nit picky to be. But I'd add a newline between end of functions and beginning of next function.
@@ -69,18 +68,11 @@ impl<R: Read> Rng for ReadRng<R> { | |||
fn try_fill(&mut self, dest: &mut [u8]) -> Result<(), Error> { | |||
if dest.len() == 0 { return Ok(()); } | |||
// Use `std::io::read_exact`, which retries on `ErrorKind::Interrupted`. | |||
self.reader.read_exact(dest).map_err(map_err) | |||
self.reader.read_exact(dest).map_err(|err| | |||
Error::new(ErrorKind::Unavailable, "ReadRng: read error", Some(err))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description doesn't really fit with the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't really sure what message to give here. Note that there should be a chained cause (though there's no guarantee it actually gets printed).
What do you think about https://www.reddit.com/r/rust/comments/7b88qp/failure_a_new_error_management_story/? |
Seems a lot like what has been in the works here. If I understand correctly, |
Looks a nice library but nothing ground-breaking.
That's kind of the problem we had; unfortunately we need some |
This replaces #30; I plan to merge shortly