Skip to content
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

rand_core::Error new version #771

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 54 additions & 139 deletions rand_core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,169 +9,84 @@
//! Error types

use core::fmt;
use core::num::NonZeroU32;

#[cfg(feature="std")]
use std::error::Error as stdError;
#[cfg(feature="std")]
use std::io;

/// Error kind which can be matched over.
#[derive(PartialEq, Eq, Debug, Copy, Clone)]
pub enum ErrorKind {
/// Feature is not available; not recoverable.
///
/// This is the most permanent failure type and implies the error cannot be
/// resolved simply by retrying (e.g. the feature may not exist in this
/// build of the application or on the current platform).
Unavailable,
/// General failure; there may be a chance of recovery on retry.
///
/// This is the catch-all kind for errors from known and unknown sources
/// which do not have a more specific kind / handling method.
///
/// It is suggested to retry a couple of times or retry later when
/// handling; some error sources may be able to resolve themselves,
/// although this is not likely.
Unexpected,
/// A transient failure which likely can be resolved or worked around.
///
/// This error kind exists for a few specific cases where it is known that
/// the error likely can be resolved internally, but is reported anyway.
Transient,
/// Not ready yet: recommended to try again a little later.
///
/// This error kind implies the generator needs more time or needs some
/// other part of the application to do something else first before it is
/// ready for use; for example this may be used by external generators
/// which require time for initialization.
NotReady,
#[doc(hidden)]
__Nonexhaustive,
}

impl ErrorKind {
/// True if this kind of error may resolve itself on retry.
///
/// See also `should_wait()`.
pub fn should_retry(self) -> bool {
self != ErrorKind::Unavailable
}

/// True if we should retry but wait before retrying
///
/// This implies `should_retry()` is true.
pub fn should_wait(self) -> bool {
self == ErrorKind::NotReady
}

/// A description of this error kind
pub fn description(self) -> &'static str {
match self {
ErrorKind::Unavailable => "permanently unavailable",
ErrorKind::Unexpected => "unexpected failure",
ErrorKind::Transient => "transient failure",
ErrorKind::NotReady => "not ready yet",
ErrorKind::__Nonexhaustive => unreachable!(),
}
}
}
/// TODO
#[derive(Debug, Copy, Clone)]
struct ErrorCode(NonZeroU32);


/// Error type of random number generators
///
/// This is a relatively simple error type, designed for compatibility with and
/// without the Rust `std` library. It embeds a "kind" code, a message (static
/// string only), and an optional chained cause (`std` only). The `kind` and
/// `msg` fields can be accessed directly; cause can be accessed via
/// `std::error::Error::cause` or `Error::take_cause`. Construction can only be
/// done via `Error::new` or `Error::with_cause`.
/// TODO
#[derive(Debug)]
pub struct Error {
/// The error kind
pub kind: ErrorKind,
/// The error message
pub msg: &'static str,
#[cfg(not(feature="std"))]
code: ErrorCode,
#[cfg(feature="std")]
cause: Option<Box<stdError + Send + Sync>>,
cause: Box<dyn std::error::Error + Send + Sync>,
}

impl Error {
/// Create a new instance, with specified kind and a message.
pub fn new(kind: ErrorKind, msg: &'static str) -> Self {
/// TODO
pub fn from_code(code: NonZeroU32) -> Self {
let code = ErrorCode(code);
#[cfg(not(feature="std"))] {
Self { code }
}
#[cfg(feature="std")] {
Error { kind, msg, cause: None }
Self { cause: Box::new(code) }
}
}

/// TODO
pub fn code(&self) -> Option<NonZeroU32> {
#[cfg(not(feature="std"))] {
Error { kind, msg }
Some(self.code.0)
}
#[cfg(feature="std")] {
self.cause.downcast_ref().map(|r: &ErrorCode| r.0)
}
}

/// Create a new instance, with specified kind, message, and a
/// chained cause.
///
/// Note: `stdError` is an alias for `std::error::Error`.
///
/// If not targetting `std` (i.e. `no_std`), this function is replaced by
/// another with the same prototype, except that there are no bounds on the
/// type `E` (because both `Box` and `stdError` are unavailable), and the
/// `cause` is ignored.
#[cfg(feature="std")]
pub fn with_cause<E>(kind: ErrorKind, msg: &'static str, cause: E) -> Self
where E: Into<Box<stdError + Send + Sync>>
{
Error { kind, msg, cause: Some(cause.into()) }
}

/// Create a new instance, with specified kind, message, and a
/// chained cause.
///
/// In `no_std` mode the *cause* is ignored.
#[cfg(not(feature="std"))]
pub fn with_cause<E>(kind: ErrorKind, msg: &'static str, _cause: E) -> Self {
Error { kind, msg }
}

/// Take the cause, if any. This allows the embedded cause to be extracted.
/// This uses `Option::take`, leaving `self` with no cause.
#[cfg(feature="std")]
pub fn take_cause(&mut self) -> Option<Box<stdError + Send + Sync>> {
self.cause.take()
}
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
#[cfg(feature="std")] {
if let Some(ref cause) = self.cause {
return write!(f, "{} ({}); cause: {}",
self.msg, self.kind.description(), cause);
}
}
write!(f, "{} ({})", self.msg, self.kind.description())
// TODO
Ok(())
}
}

#[cfg(feature="std")]
impl stdError for Error {
fn description(&self) -> &str {
self.msg
}

fn cause(&self) -> Option<&stdError> {
self.cause.as_ref().map(|e| e.as_ref() as &stdError)
impl fmt::Display for ErrorCode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// TODO
Ok(())
}
}

#[cfg(feature="std")]
impl From<Error> for io::Error {
fn from(error: Error) -> Self {
use std::io::ErrorKind::*;
match error.kind {
ErrorKind::Unavailable => io::Error::new(NotFound, error),
ErrorKind::Unexpected |
ErrorKind::Transient => io::Error::new(Other, error),
ErrorKind::NotReady => io::Error::new(WouldBlock, error),
ErrorKind::__Nonexhaustive => unreachable!(),
mod std_impls {
use std::error::Error as StdError;
use std::io;
use super::{Error, ErrorCode};

impl Error {
/// TODO
pub fn from_cause<E>(cause: E) -> Self
where E: Into<Box<dyn StdError + Send + Sync>>
{
Self { cause: cause.into() }
}

/// TODO
pub fn cause(self) -> Box<dyn StdError + Send + Sync> {
self.cause
}
}

impl StdError for ErrorCode { }
impl StdError for Error { }

impl From<Error> for io::Error {
fn from(error: Error) -> Self {
io::Error::new(io::ErrorKind::Other, error)
}
}
}
2 changes: 1 addition & 1 deletion rand_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use core::ptr::copy_nonoverlapping;

#[cfg(all(feature="alloc", not(feature="std")))] use alloc::boxed::Box;

pub use error::{ErrorKind, Error};
pub use error::Error;


mod error;
Expand Down
12 changes: 0 additions & 12 deletions rand_jitter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
// <LICENSE-MIT or https://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rand_core::{Error, ErrorKind};
use core::fmt;

/// An error that can occur when [`JitterRng::test_timer`] fails.
Expand Down Expand Up @@ -54,13 +52,3 @@ impl ::std::error::Error for TimerError {
self.description()
}
}

impl From<TimerError> for Error {
fn from(err: TimerError) -> Error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want this (and it's also not incompatible with the current Error type).

// Timer check is already quite permissive of failures so we don't
// expect false-positive failures, i.e. any error is irrecoverable.
Error::with_cause(ErrorKind::Unavailable,
"timer jitter failed basic quality tests", err)
}
}

5 changes: 3 additions & 2 deletions rand_jitter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,11 @@ impl JitterRng {
/// # Example
///
/// ```
/// # use rand_jitter::rand_core::{RngCore, Error};
/// # use rand_jitter::rand_core::RngCore;
/// # use rand_jitter::TimerError;
/// use rand_jitter::JitterRng;
///
/// # fn try_inner() -> Result<(), Error> {
/// # fn try_inner() -> Result<(), TimerError> {
/// fn get_nstime() -> u64 {
/// use std::time::{SystemTime, UNIX_EPOCH};
///
Expand Down
1 change: 1 addition & 0 deletions rand_os/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ travis-ci = { repository = "rust-random/rand" }
appveyor = { repository = "rust-random/rand" }

[features]
std = ["rand_core/std", "getrandom/std"]
log = ["getrandom/log"]
# re-export optional WASM dependencies to avoid breakage:
wasm-bindgen = ["getrandom/wasm-bindgen"]
Expand Down
20 changes: 10 additions & 10 deletions rand_os/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,18 @@
#![deny(missing_debug_implementations)]
#![doc(test(attr(allow(unused_variables), deny(warnings))))]

#![cfg_attr(feature = "stdweb", recursion_limit="128")]

#![no_std] // but see getrandom crate

pub use rand_core; // re-export

use getrandom::getrandom;
use rand_core::{CryptoRng, RngCore, Error, ErrorKind, impls};
use rand_core::{CryptoRng, RngCore, Error, impls};

/// A random number generator that retrieves randomness from from the
/// operating system.
///
/// This is a zero-sized struct. It can be freely constructed with `OsRng`.
///
///
/// The implementation is provided by the [getrandom] crate. Refer to
/// [getrandom] documentation for details.
///
Expand Down Expand Up @@ -84,12 +82,14 @@ impl RngCore for OsRng {
}

fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
// Some systems do not support reading 0 random bytes.
// (And why waste a system call?)
if dest.len() == 0 { return Ok(()); }

getrandom(dest).map_err(|e|
Error::with_cause(ErrorKind::Unavailable, "OsRng failed", e))
getrandom(dest).map_err(|e| {
#[cfg(feature="std")] {
rand_core::Error::from_cause(e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nicer not to have to use a cfg here (e.g. make from_code work either way). That means the std feature is also not needed on this crate.

}
#[cfg(not(feature="std"))] {
rand_core::Error::from_code(e.code())
}
})
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ extern crate rand_pcg;


// Re-exports from rand_core
pub use rand_core::{RngCore, CryptoRng, SeedableRng};
pub use rand_core::{ErrorKind, Error};
pub use rand_core::{RngCore, CryptoRng, SeedableRng, Error};

// Public exports
#[cfg(feature="std")] pub use rngs::thread::thread_rng;
Expand Down
24 changes: 12 additions & 12 deletions src/rngs/adapter/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

use std::io::Read;

use rand_core::{RngCore, Error, ErrorKind, impls};
use rand_core::{RngCore, Error, impls};


/// An RNG that reads random bytes straight from any type supporting
Expand Down Expand Up @@ -73,22 +73,15 @@ impl<R: Read> RngCore for ReadRng<R> {
fn try_fill_bytes(&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(|err| {
match err.kind() {
::std::io::ErrorKind::UnexpectedEof => Error::with_cause(
ErrorKind::Unavailable,
"not enough bytes available, reached end of source", err),
_ => Error::with_cause(ErrorKind::Unavailable,
"error reading from Read source", err)
}
})
self.reader.read_exact(dest).map_err(Error::from_cause)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory it's nice to be able to include an extra message here (error manifesting and cause), but not critical.

}
}

#[cfg(test)]
mod test {
use super::ReadRng;
use {RngCore, ErrorKind};
use RngCore;
use std::io;

#[test]
fn test_reader_rng_u64() {
Expand Down Expand Up @@ -131,6 +124,13 @@ mod test {

let mut rng = ReadRng::new(&v[..]);

assert!(rng.try_fill_bytes(&mut w).err().unwrap().kind == ErrorKind::Unavailable);
let err_kind = rng.try_fill_bytes(&mut w)
.err()
.unwrap()
.cause()
.downcast_ref()
.map(|e: &io::Error| e.kind())
.unwrap();
assert_eq!(err_kind, io::ErrorKind::UnexpectedEof);
dhardy marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading