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

Revise rand_core::Error (alt #771) #800

Merged
merged 10 commits into from May 29, 2019
1 change: 1 addition & 0 deletions .travis.yml
Expand Up @@ -268,6 +268,7 @@ before_install:

script:
- cargo test --tests --no-default-features
- cargo test --no-default-features --features getrandom
# TODO: add simd_support feature:
- cargo test --features=serde1,log
- cargo test --examples
Expand Down
8 changes: 5 additions & 3 deletions Cargo.toml
Expand Up @@ -28,8 +28,9 @@ i128_support = [] # enables i128 and u128 support
simd_support = ["packed_simd"] # enables SIMD support
serde1 = ["rand_core/serde1", "rand_isaac/serde1", "rand_xorshift/serde1"] # enables serialization for PRNGs
# re-export optional WASM dependencies to avoid breakage:
wasm-bindgen = ["getrandom/wasm-bindgen"]
stdweb = ["getrandom/stdweb"]
wasm-bindgen = ["getrandom_package/wasm-bindgen"]
stdweb = ["getrandom_package/stdweb"]
getrandom = ["getrandom_package", "rand_core/getrandom"]

[workspace]
members = [
Expand All @@ -50,7 +51,8 @@ members = [
rand_core = { path = "rand_core", version = "0.4" }
rand_pcg = { path = "rand_pcg", version = "0.1" }
rand_hc = { path = "rand_hc", version = "0.1" }
getrandom = { version = "0.1.1", optional = true }
# Do not depend on 'getrandom_package' directly; use the 'getrandom' feature!
getrandom_package = { version = "0.1.1", package = "getrandom", optional = true }
log = { version = "0.4", optional = true }

[dependencies.packed_simd]
Expand Down
3 changes: 2 additions & 1 deletion rand_core/Cargo.toml
Expand Up @@ -18,10 +18,11 @@ travis-ci = { repository = "rust-random/rand" }
appveyor = { repository = "rust-random/rand" }

[features]
std = ["alloc"] # use std library; should be default but for above bug
std = ["alloc", "getrandom", "getrandom/std"] # use std library; should be default but for above bug
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this coupled to getrandom?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean we should support std without getrandom? We could try, but (1) getrandom supports (pretty-much) any std platform anyway and (2) I'm not sure Cargo allows us to imply getrandom/std only when both std and getrandom are enabled.

alloc = [] # enables Vec and Box support without std
serde1 = ["serde", "serde_derive"] # enables serde for BlockRng wrapper

[dependencies]
serde = { version = "1", optional = true }
serde_derive = { version = "^1.0.38", optional = true }
getrandom = { version = "0.1", optional = true }
213 changes: 85 additions & 128 deletions rand_core/src/error.rs
Expand Up @@ -9,169 +9,126 @@
//! Error types

use core::fmt;

#[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!(),
}
}
}
use core::num::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`.
///
/// In order to be compatible with `std` and `no_std`, this type has two
/// possible implementations: with `std` a boxed `Error` trait object is stored,
/// while with `no_std` we merely store an error code.
#[derive(Debug)]
pub struct Error {
/// The error kind
pub kind: ErrorKind,
/// The error message
pub msg: &'static str,
#[cfg(feature="std")]
cause: Option<Box<stdError + Send + Sync>>,
inner: Box<dyn std::error::Error + Send + Sync + 'static>,
#[cfg(not(feature="std"))]
code: NonZeroU32,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this still have the issue that the error types are incompatible for different features, making the std feature non-additive?

Copy link
Member

Choose a reason for hiding this comment

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

No, with enabled std feature NonZeroU32 code gets wrapped into ErrorCode, so public API gets only extended with new methods and no_std part of API does not change in any way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Functionally this is the case (and the code method can no longer guarantee to have Some(NonZeroU32) result), but API changes are only additive.


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

/// Create a new instance, with specified kind, message, and a
/// chained cause.
/// Construct from any type supporting `std::error::Error`
///
/// Note: `stdError` is an alias for `std::error::Error`.
/// Available only when configured with `std`.
///
/// 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.
/// See also `From<NonZeroU32>`, which is available with and without `std`.
#[cfg(feature="std")]
pub fn with_cause<E>(kind: ErrorKind, msg: &'static str, cause: E) -> Self
where E: Into<Box<stdError + Send + Sync>>
pub fn new<E>(err: E) -> Self
where E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>
{
Error { kind, msg, cause: Some(cause.into()) }
Error { inner: err.into() }
}

/// Create a new instance, with specified kind, message, and a
/// chained cause.
/// Reference the inner error (`std` only)
///
/// 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 }
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
pub fn inner(&self) -> &(dyn std::error::Error + Send + Sync + 'static) {
&*self.inner
}

/// Take the cause, if any. This allows the embedded cause to be extracted.
/// This uses `Option::take`, leaving `self` with no cause.
/// Unwrap the inner error (`std` only)
///
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
pub fn take_cause(&mut self) -> Option<Box<stdError + Send + Sync>> {
self.cause.take()
pub fn take_inner(self) -> Box<dyn std::error::Error + Send + Sync + 'static> {
self.inner
}

/// Retrieve the error code, if any.
///
/// If this `Error` was constructed via `From<NonZeroU32>`, then this method
/// will return this `NonZeroU32` code (for `no_std` this is always the
/// case). Otherwise, this method will return `None`.
pub fn code(&self) -> Option<NonZeroU32> {
#[cfg(feature="std")] {
self.inner.downcast_ref::<ErrorCode>().map(|c| c.0)
}
#[cfg(not(feature="std"))] {
Some(self.code)
}
}
}

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.inner)
}
#[cfg(not(feature="std"))] {
write!(f, "error code {}", self.code)
}
write!(f, "{} ({})", self.msg, self.kind.description())
}
}

#[cfg(feature="std")]
impl stdError for Error {
fn description(&self) -> &str {
self.msg
impl From<NonZeroU32> for Error {
fn from(code: NonZeroU32) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(ErrorCode(code)) }
}
#[cfg(not(feature="std"))] {
Error { code }
}
}
}

fn cause(&self) -> Option<&stdError> {
self.cause.as_ref().map(|e| e.as_ref() as &stdError)
#[cfg(feature="getrandom")]
impl From<getrandom::Error> for Error {
fn from(error: getrandom::Error) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(error) }
}
#[cfg(not(feature="std"))] {
Error { code: error.code() }
}
}
}

#[cfg(feature="std")]
impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.inner.source()
}
}

#[cfg(feature="std")]
impl From<Error> for io::Error {
impl From<Error> for std::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!(),
}
std::io::Error::new(std::io::ErrorKind::Other, error)
}
}

#[cfg(feature="std")]
#[derive(Debug, Copy, Clone)]
struct ErrorCode(NonZeroU32);

#[cfg(feature="std")]
impl fmt::Display for ErrorCode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "error code {}", self.0)
}
}

#[cfg(feature="std")]
impl std::error::Error for ErrorCode {}