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
113 changes: 52 additions & 61 deletions rand_core/src/error.rs
Expand Up @@ -12,78 +12,58 @@ use core::fmt;
use core::num::NonZeroU32;


// A randomly-chosen 24-bit prefix for our codes.
#[cfg(not(feature="std"))]
pub(crate) const CODE_PREFIX: u32 = 0x517e8100;

/// 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 message (static
/// string only), and an optional chained cause (`std` only). The
/// `msg` field 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 {
msg: &'static str,
#[cfg(feature="std")]
cause: Option<Box<std::error::Error + 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 a message.
pub fn new(msg: &'static str) -> Self {
#[cfg(feature="std")] {
Error { msg, cause: None }
}
#[cfg(not(feature="std"))] {
Error { msg, code: NonZeroU32::new(CODE_PREFIX).unwrap() }
}
}

/// Create a new instance, with a message and a chained cause.
///
/// This function is only available with the `std` feature.
// NOTE: with specialisation we could support both.
/// Construct from any type supporting `std::error::Error`
///
/// Available only when configured with `std`.
///
/// See also `From<NonZeroU32>`, which is available with and without `std`.
#[cfg(feature="std")]
pub fn with_cause<E>(msg: &'static str, cause: E) -> Self
where E: Into<Box<std::error::Error + Send + Sync>>
pub fn new<E>(err: E) -> Self
where E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>
{
Error { msg, cause: Some(cause.into()) }
Error { inner: err.into() }
}

/// Create a new instance, with a message and an error code.
pub fn with_code(msg: &'static str, code: NonZeroU32) -> Self {
#[cfg(feature="std")] {
Error { msg, cause: Some(Box::new(ErrorCode(code))) }
}
#[cfg(not(feature="std"))] {
Error { msg, code }
}
}

/// Retrieve the error message.
pub fn msg(&self) -> &str {
self.msg
/// Reference 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 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.
///
/// This function is only available with the `std` feature.
/// 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<std::error::Error + 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.
#[cfg(not(feature="std"))]
///
/// 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.cause.as_ref().and_then(|b| b.downcast_ref::<ErrorCode>()).map(|c| c.0)
self.inner.downcast_ref::<ErrorCode>().map(|c| c.0)
}
#[cfg(not(feature="std"))] {
Some(self.code)
Expand All @@ -93,31 +73,42 @@ impl Error {

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

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 }
}
}
}

#[cfg(feature="getrandom")]
impl From<getrandom::Error> for Error {
fn from(error: getrandom::Error) -> Self {
let msg = "getrandom error";
#[cfg(feature="std")] {
Error { msg, cause: Some(Box::new(error)) }
Error { inner: Box::new(error) }
}
#[cfg(not(feature="std"))] {
Error { msg, code: error.code() }
Error { code: error.code() }
}
}
}

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

fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.cause.as_ref().map(|e| e.as_ref() as &std::error::Error)
self.inner.source()
}
}

Expand All @@ -135,7 +126,7 @@ struct ErrorCode(NonZeroU32);
#[cfg(feature="std")]
impl fmt::Display for ErrorCode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "ErrorCode({})", self.0)
write!(f, "error code {}", self.0)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/rngs/adapter/mod.rs
Expand Up @@ -8,8 +8,8 @@

//! Wrappers / adapters forming RNGs

#[cfg(feature="std")] #[doc(hidden)] pub mod read;
#[cfg(feature="std")] mod read;
mod reseeding;

#[cfg(feature="std")] pub use self::read::ReadRng;
#[cfg(feature="std")] pub use self::read::{ReadRng, ReadError};
pub use self::reseeding::ReseedingRng;
25 changes: 22 additions & 3 deletions src/rngs/adapter/read.rs
Expand Up @@ -10,6 +10,7 @@
//! A wrapper around any Read to treat it as an RNG.

use std::io::Read;
use std::fmt;

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

Expand Down Expand Up @@ -73,11 +74,27 @@ 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|
Error::with_cause("error reading from Read source", err))
self.reader.read_exact(dest).map_err(|e| Error::new(ReadError(e)))
}
}

/// `ReadRng` error type
#[derive(Debug)]
pub struct ReadError(std::io::Error);

impl fmt::Display for ReadError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "ReadError: {}", self.0)
}
}

impl std::error::Error for ReadError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
Some(&self.0)
}
}


#[cfg(test)]
mod test {
use super::ReadRng;
Expand Down Expand Up @@ -124,6 +141,8 @@ mod test {

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

assert!(rng.try_fill_bytes(&mut w).is_err());
let result = rng.try_fill_bytes(&mut w);
assert!(result.is_err());
println!("Error: {}", result.unwrap_err());
}
}