Merge pull request #255 from dhardy/failure

ErrorKind: rename Transient → Unexpected and revise uses of Unavailable

src/error.rs

@@ -18,15 +18,33 @@ use std::error::Error as stdError;
 /// Error kind which can be matched over.
 #[derive(PartialEq, Eq, Debug, Copy, Clone)]
 pub enum ErrorKind {
-    /// Permanent failure: likely not recoverable without user action.
+    /// 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,
-    /// Temporary failure: recommended to retry a few times, but may also be
-    /// irrecoverable.
+    /// 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,
-    /// Uncategorised error
-    Other,
     #[doc(hidden)]
     __Nonexhaustive,
 }
@@ -36,10 +54,7 @@ impl ErrorKind {
     /// 
     /// See also `should_wait()`.
     pub fn should_retry(self) -> bool {
-        match self {
-            ErrorKind::Transient | ErrorKind::NotReady => true,
-            _ => false,
-        }
+        self != ErrorKind::Unavailable
     }
 
     /// True if we should retry but wait before retrying
@@ -52,24 +67,30 @@ impl ErrorKind {
     /// A description of this error kind
     pub fn description(self) -> &'static str {
         match self {
-            ErrorKind::Unavailable => "permanent failure",
+            ErrorKind::Unavailable => "permanently unavailable",
+            ErrorKind::Unexpected => "unexpected failure",
             ErrorKind::Transient => "transient failure",
             ErrorKind::NotReady => "not ready yet",
-            ErrorKind::Other => "uncategorised",
             ErrorKind::__Nonexhaustive => unreachable!(),
         }
     }
 }
 
+
 /// 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).
+/// 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`.
 #[derive(Debug)]
 pub struct Error {
-    kind: ErrorKind,
-    msg: &'static str,
+    /// The error kind
+    pub kind: ErrorKind,
+    /// The error message
+    pub msg: &'static str,
     #[cfg(feature="std")]
     cause: Option<Box<stdError + Send + Sync>>,
 }
@@ -110,14 +131,11 @@ impl Error {
         Error { kind: kind, msg: msg }
     }
 
-    /// Get the error kind
-    pub fn kind(&self) -> ErrorKind {
-        self.kind
-    }
-
-    /// Get the error message
-    pub fn msg(&self) -> &'static str {
-        self.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()
     }
 }
 
@@ -126,12 +144,10 @@ impl fmt::Display for Error {
         #[cfg(feature="std")] {
             if let Some(ref cause) = self.cause {
                 return write!(f, "{} ({}); cause: {}",
-                        self.msg(),
-                        self.kind.description(),
-                        cause);
+                        self.msg, self.kind.description(), cause);
             }
         }
-        write!(f, "{} ({})", self.msg(), self.kind.description())
+        write!(f, "{} ({})", self.msg, self.kind.description())
     }
 }
 

src/jitter.rs

@@ -145,6 +145,8 @@ impl ::std::error::Error for TimerError {
 
 impl From<TimerError> for Error {
     fn from(err: TimerError) -> Error {
+        // 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)
     }

src/os.rs

@@ -82,31 +82,27 @@ impl RngCore for OsRng {
                     panic!("OsRng failed too many times; last error: {}", e);
                 }
 
-                match e.kind() {
-                    ErrorKind::Transient => {
-                        if !error_logged {
-                            warn!("OsRng failed; retrying up to {} times. Error: {}",
-                                    TRANSIENT_RETRIES, e);
-                            error_logged = true;
-                        }
-                        err_count += (RETRY_LIMIT + TRANSIENT_RETRIES - 1)
-                                / TRANSIENT_RETRIES;    // round up
-                        continue;
-                    }
-                    ErrorKind::NotReady => {
-                        if !error_logged {
-                            warn!("OsRng failed; waiting up to {}s and retrying. Error: {}",
-                                    MAX_RETRY_PERIOD, e);
-                            error_logged = true;
-                        }
-                        err_count += 1;
-                        thread::sleep(wait_dur);
-                        continue;
+                if e.kind.should_wait() {
+                    if !error_logged {
+                        warn!("OsRng failed; waiting up to {}s and retrying. Error: {}",
+                                MAX_RETRY_PERIOD, e);
+                        error_logged = true;
                     }
-                    _ => {
-                        error!("OsRng failed: {}", e);
-                        panic!("OsRng fatal error: {}", e);
+                    err_count += 1;
+                    thread::sleep(wait_dur);
+                    continue;
+                } else if e.kind.should_retry() {
+                    if !error_logged {
+                        warn!("OsRng failed; retrying up to {} times. Error: {}",
+                                TRANSIENT_RETRIES, e);
+                        error_logged = true;
                     }
+                    err_count += (RETRY_LIMIT + TRANSIENT_RETRIES - 1)
+                            / TRANSIENT_RETRIES;    // round up
+                    continue;
+                } else {
+                    error!("OsRng failed: {}", e);
+                    panic!("OsRng fatal error: {}", e);
                 }
             }
 
@@ -147,11 +143,16 @@ impl ReadRng {
         let mut guard = mutex.lock().unwrap();
         if (*guard).is_none() {
             info!("OsRng: opening random device {}", path.as_ref().display());
-            let file = File::open(path).map_err(|err| Error::with_cause(
-                    ErrorKind::Unavailable,
-                    "error opening random device",
-                    err
-            ))?;
+            let file = File::open(path).map_err(|err| {
+                use std::io::ErrorKind::*;
+                match err.kind() {
+                    Interrupted => Error::new(ErrorKind::Transient, "interrupted"),
+                    WouldBlock => Error::with_cause(ErrorKind::NotReady,
+                            "opening random device would block", err),
+                    _ => Error::with_cause(ErrorKind::Unavailable,
+                            "error while opening random device", err)
+                }
+            })?;
             *guard = Some(file);
         }
 
@@ -169,7 +170,13 @@ impl ReadRng {
         let mut file = (*guard).as_mut().unwrap();
         // Use `std::io::read_exact`, which retries on `ErrorKind::Interrupted`.
         file.read_exact(dest).map_err(|err| {
-            Error::with_cause(ErrorKind::Unavailable, "error reading random device", err)
+            match err.kind() {
+                ::std::io::ErrorKind::WouldBlock => Error::with_cause(
+                    ErrorKind::NotReady,
+                    "reading from random device would block", err),
+                _ => Error::with_cause(ErrorKind::Unavailable,
+                    "error reading random device", err)
+            }
         })
     }
 }

src/read.rs

@@ -62,7 +62,13 @@ impl<R: Read> RngCore for ReadRng<R> {
         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(ErrorKind::Unavailable, "ReadRng: read error", err)
+            match err.kind() {
+                ::std::io::ErrorKind::WouldBlock => Error::with_cause(
+                    ErrorKind::NotReady,
+                    "reading from random device would block", err),
+                _ => Error::with_cause(ErrorKind::Unavailable,
+                    "error reading random device", err)
+            }
         })
     }
 }

src/reseeding.rs

@@ -83,63 +83,43 @@ impl<R: RngCore + SeedableRng, Rsdr: RngCore> ReseedingRng<R, Rsdr> {
     /// Reseed the internal PRNG.
     ///
     /// This will try to work around errors in the RNG used for reseeding
-    /// intelligently. If the error kind indicates retrying might help, it will
-    /// immediately retry a couple of times. If the error kind indicates the
-    /// seeding RNG is not ready, it will retry later, after `threshold / 256`
-    /// generated bytes. On other errors in the source RNG, this will skip
-    /// reseeding and continue using the internal PRNG, until another
-    /// `threshold` bytes have been generated (at which point it will try
-    /// reseeding again).
-    #[inline(never)]
+    /// intelligently through some combination of retrying and delaying
+    /// reseeding until later. So long as the internal PRNG doesn't fail, this
+    /// method will not fail, i.e. failures from the reseeding source are not
+    /// fatal.
     pub fn reseed(&mut self) {
-        trace!("Reseeding RNG after generating {} bytes",
-               self.threshold - self.bytes_until_reseed);
-        self.bytes_until_reseed = self.threshold;
-        let mut err_count = 0;
-        loop {
-            if let Err(e) = R::from_rng(&mut self.reseeder)
-                            .map(|result| self.rng = result) {
-                let kind = e.kind();
-                if kind.should_wait() {
-                    self.bytes_until_reseed = self.threshold >> 8;
-                    warn!("Reseeding RNG delayed for {} bytes",
-                           self.bytes_until_reseed);
-                } else if kind.should_retry() {
-                    err_count += 1;
-                    // Retry immediately for 5 times (arbitrary limit)
-                    if err_count <= 5 { continue; }
-                }
-                warn!("Reseeding RNG failed; continuing without reseeding. Error: {}", e);
-            }
-            break; // Successfully reseeded, delayed, or given up.
-        }
+        // Behaviour is identical to `try_reseed`; we just squelch the error.
+        let _res = self.try_reseed();
     }
 
     /// Reseed the internal RNG if the number of bytes that have been
     /// generated exceed the threshold.
     ///
     /// If reseeding fails, return an error with the original cause. Note that
-    /// if the cause has a permanent failure, we report a transient error and
-    /// skip reseeding; this means that only two error kinds can be reported
-    /// from this method: `ErrorKind::Transient` and `ErrorKind::NotReady`.
+    /// in case of error we simply delay reseeding, allowing the generator to
+    /// continue its output of random data and try reseeding again later;
+    /// because of this we always return kind `ErrorKind::Transient`.
     #[inline(never)]
     pub fn try_reseed(&mut self) -> Result<(), Error> {
         trace!("Reseeding RNG after {} generated bytes",
                self.threshold - self.bytes_until_reseed);
-        if let Err(err) = R::from_rng(&mut self.reseeder)
-                          .map(|result| self.rng = result) {
-            let newkind = match err.kind() {
-                a @ ErrorKind::NotReady => a,
-                b @ ErrorKind::Transient => b,
-                _ => {
-                    self.bytes_until_reseed = self.threshold; // skip reseeding
-                    ErrorKind::Transient
-                }
+        if let Err(mut e) = R::from_rng(&mut self.reseeder)
+                .map(|result| self.rng = result)
+        {
+            let delay = match e.kind {
+                ErrorKind::Transient => 0,
+                kind @ _ if kind.should_retry() => self.threshold >> 8,
+                _ => self.threshold,
             };
-            return Err(Error::with_cause(newkind, "reseeding failed", err));
+            warn!("Reseeding RNG delayed reseeding by {} bytes due to \
+                    error from source: {}", delay, e);
+            self.bytes_until_reseed = delay;
+            e.kind = ErrorKind::Transient;
+            Err(e)
+        } else {
+            self.bytes_until_reseed = self.threshold;
+            Ok(())
         }
-        self.bytes_until_reseed = self.threshold;
-        Ok(())
     }
 }
 
@@ -171,12 +151,21 @@ impl<R: RngCore + SeedableRng, Rsdr: RngCore> RngCore for ReseedingRng<R, Rsdr>
     }
 
     fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
-        self.rng.try_fill_bytes(dest)?;
+        let res1 = self.rng.try_fill_bytes(dest);
         self.bytes_until_reseed -= dest.len() as i64;
-        if self.bytes_until_reseed <= 0 {
-            self.try_reseed()?;
+        let res2 = if self.bytes_until_reseed <= 0 {
+            self.try_reseed()
+        } else { Ok(()) };
+
+        if let Err(e) = res1 {
+            // In the unlikely event the internal PRNG fails, we don't know
+            // whether this is resolvable; reseed immediately and return
+            // original error kind.
+            self.bytes_until_reseed = 0;
+            Err(e)
+        } else {
+            res2
         }
-        Ok(())
     }
 }