Improve Error handling (#54)

Author: josephlr

Cargo.toml

@@ -22,7 +22,7 @@ log = { version = "0.4", optional = true }
 cfg-if = "0.1"
 
 [target.'cfg(any(unix, target_os = "redox", target_os = "wasi"))'.dependencies]
-libc = "0.2.54"
+libc = "0.2.60"
 
 [target.wasm32-unknown-unknown.dependencies]
 wasm-bindgen = { version = "0.2.29", optional = true }

src/cloudabi.rs

@@ -17,14 +17,9 @@ extern "C" {
 pub fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> {
     let errno = unsafe { cloudabi_sys_random_get(dest.as_mut_ptr(), dest.len()) };
     if let Some(code) = NonZeroU32::new(errno as u32) {
-        error!("cloudabi_sys_random_get failed with code {}", code);
+        error!("cloudabi_sys_random_get: failed with {}", errno);
         Err(Error::from(code))
     } else {
         Ok(()) // Zero means success for CloudABI
     }
 }
-
-#[inline(always)]
-pub fn error_msg_inner(_: NonZeroU32) -> Option<&'static str> {
-    None
-}

src/dummy.rs

@@ -6,17 +6,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! A dummy implementation for unsupported targets which always returns
-//! `Err(Error::UNAVAILABLE)`
-use crate::Error;
-use core::num::NonZeroU32;
+//! A dummy implementation for unsupported targets which always fails
+use crate::{error::UNSUPPORTED, Error};
 
 pub fn getrandom_inner(_: &mut [u8]) -> Result<(), Error> {
-    error!("no support for this platform");
-    Err(Error::UNAVAILABLE)
-}
-
-#[inline(always)]
-pub fn error_msg_inner(_: NonZeroU32) -> Option<&'static str> {
-    None
+    Err(UNSUPPORTED)
 }

src/error.rs

@@ -5,78 +5,151 @@
 // <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 core::convert::From;
 use core::fmt;
 use core::num::NonZeroU32;
 
-// A randomly-chosen 24-bit prefix for our codes
-pub(crate) const CODE_PREFIX: u32 = 0x57f4c500;
-const CODE_UNKNOWN: u32 = CODE_PREFIX | 0x00;
-const CODE_UNAVAILABLE: u32 = CODE_PREFIX | 0x01;
-
-/// The error type.
+/// A small and `no_std` compatible error type.
+///
+/// The [`Error::raw_os_error()`] will indicate if the error is from the OS, and
+/// if so, which error code the OS gave the application. If such an error is
+/// encountered, please consult with your system documentation.
 ///
-/// This type is small and no-std compatible.
+/// Internally this type is a NonZeroU32, with certain values reserved for
+/// certain purposes, see [`Error::INTERNAL_START`] and [`Error::CUSTOM_START`].
 #[derive(Copy, Clone, Eq, PartialEq)]
-pub struct Error(pub(crate) NonZeroU32);
+pub struct Error(NonZeroU32);
 
 impl Error {
-    /// An unknown error.
-    pub const UNKNOWN: Error = Error(unsafe { NonZeroU32::new_unchecked(CODE_UNKNOWN) });
+    #[deprecated(since = "0.1.7")]
+    pub const UNKNOWN: Error = UNSUPPORTED;
+    #[deprecated(since = "0.1.7")]
+    pub const UNAVAILABLE: Error = UNSUPPORTED;
+
+    /// Codes below this point represent OS Errors (i.e. positive i32 values).
+    /// Codes at or above this point, but below [`Error::CUSTOM_START`] are
+    /// reserved for use by the `rand` and `getrandom` crates.
+    pub const INTERNAL_START: u32 = 1 << 31;
 
-    /// No generator is available.
-    pub const UNAVAILABLE: Error = Error(unsafe { NonZeroU32::new_unchecked(CODE_UNAVAILABLE) });
+    /// Codes at or above this point can be used by users to define their own
+    /// custom errors.
+    pub const CUSTOM_START: u32 = (1 << 31) + (1 << 30);
 
-    /// Extract the error code.
+    /// Extract the raw OS error code (if this error came from the OS)
     ///
-    /// This may equal one of the codes defined in this library or may be a
-    /// system error code.
+    /// This method is identical to `std::io::Error::raw_os_error()`, except
+    /// that it works in `no_std` contexts. If this method returns `None`, the
+    /// error value can still be formatted via the `Diplay` implementation.
+    #[inline]
+    pub fn raw_os_error(&self) -> Option<i32> {
+        if self.0.get() < Self::INTERNAL_START {
+            Some(self.0.get() as i32)
+        } else {
+            None
+        }
+    }
+
+    /// Extract the bare error code.
     ///
-    /// One may attempt to format this error via the `Display` implementation.
+    /// This code can either come from the underlying OS, or be a custom error.
+    /// Use [`Error::raw_os_error()`] to disambiguate.
+    #[inline]
     pub fn code(&self) -> NonZeroU32 {
         self.0
     }
+}
 
-    pub(crate) fn msg(&self) -> Option<&'static str> {
-        if let Some(msg) = crate::imp::error_msg_inner(self.0) {
-            Some(msg)
-        } else {
-            match *self {
-                Error::UNKNOWN => Some("getrandom: unknown error"),
-                Error::UNAVAILABLE => Some("getrandom: unavailable"),
-                _ => None,
-            }
-        }
+#[cfg(any(unix, target_os = "redox"))]
+fn os_err_desc(errno: i32, buf: &mut [u8]) -> Option<&str> {
+    let buf_ptr = buf.as_mut_ptr() as *mut libc::c_char;
+    if unsafe { libc::strerror_r(errno, buf_ptr, buf.len()) } != 0 {
+        return None;
     }
+
+    // Take up to trailing null byte
+    let idx = buf.iter().position(|&b| b == 0).unwrap_or(buf.len());
+    core::str::from_utf8(&buf[..idx]).ok()
+}
+
+#[cfg(not(any(unix, target_os = "redox")))]
+fn os_err_desc(_errno: i32, _buf: &mut [u8]) -> Option<&str> {
+    None
 }
 
 impl fmt::Debug for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        match self.msg() {
-            Some(msg) => write!(f, "Error(\"{}\")", msg),
-            None => write!(f, "Error(0x{:08X})", self.0),
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let mut dbg = f.debug_struct("Error");
+        if let Some(errno) = self.raw_os_error() {
+            dbg.field("os_error", &errno);
+            let mut buf = [0u8; 128];
+            if let Some(desc) = os_err_desc(errno, &mut buf) {
+                dbg.field("description", &desc);
+            }
+        } else if let Some(desc) = internal_desc(*self) {
+            dbg.field("internal_code", &self.0.get());
+            dbg.field("description", &desc);
+        } else {
+            dbg.field("unknown_code", &self.0.get());
         }
+        dbg.finish()
     }
 }
 
 impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        match self.msg() {
-            Some(msg) => write!(f, "{}", msg),
-            None => write!(f, "getrandom: unknown code 0x{:08X}", self.0),
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if let Some(errno) = self.raw_os_error() {
+            let mut buf = [0u8; 128];
+            match os_err_desc(errno, &mut buf) {
+                Some(desc) => f.write_str(desc),
+                None => write!(f, "OS Error: {}", errno),
+            }
+        } else if let Some(desc) = internal_desc(*self) {
+            f.write_str(desc)
+        } else {
+            write!(f, "Unknown Error: {}", self.0.get())
         }
     }
 }
 
 impl From<NonZeroU32> for Error {
     fn from(code: NonZeroU32) -> Self {
-        Error(code)
+        Self(code)
     }
 }
 
-impl From<&Error> for Error {
-    fn from(error: &Error) -> Self {
-        *error
+// TODO: Convert to a function when min_version >= 1.33
+macro_rules! internal_error {
+    ($n:expr) => {
+        Error(unsafe { NonZeroU32::new_unchecked(Error::INTERNAL_START + $n as u16 as u32) })
+    };
+}
+
+/// Internal Error constants
+pub(crate) const UNSUPPORTED: Error = internal_error!(0);
+pub(crate) const ERRNO_NOT_POSITIVE: Error = internal_error!(1);
+pub(crate) const UNKNOWN_IO_ERROR: Error = internal_error!(2);
+pub(crate) const SEC_RANDOM_FAILED: Error = internal_error!(3);
+pub(crate) const RTL_GEN_RANDOM_FAILED: Error = internal_error!(4);
+pub(crate) const FAILED_RDRAND: Error = internal_error!(5);
+pub(crate) const NO_RDRAND: Error = internal_error!(6);
+pub(crate) const BINDGEN_CRYPTO_UNDEF: Error = internal_error!(7);
+pub(crate) const BINDGEN_GRV_UNDEF: Error = internal_error!(8);
+pub(crate) const STDWEB_NO_RNG: Error = internal_error!(9);
+pub(crate) const STDWEB_RNG_FAILED: Error = internal_error!(10);
+
+fn internal_desc(error: Error) -> Option<&'static str> {
+    match error {
+        UNSUPPORTED => Some("getrandom: this target is not supported"),
+        ERRNO_NOT_POSITIVE => Some("errno: did not return a positive value"),
+        UNKNOWN_IO_ERROR => Some("Unknown std::io::Error"),
+        SEC_RANDOM_FAILED => Some("SecRandomCopyBytes: call failed"),
+        RTL_GEN_RANDOM_FAILED => Some("RtlGenRandom: call failed"),
+        FAILED_RDRAND => Some("RDRAND: failed multiple times: CPU issue likely"),
+        NO_RDRAND => Some("RDRAND: instruction not supported"),
+        BINDGEN_CRYPTO_UNDEF => Some("wasm-bindgen: self.crypto is undefined"),
+        BINDGEN_GRV_UNDEF => Some("wasm-bindgen: crypto.getRandomValues is undefined"),
+        STDWEB_NO_RNG => Some("stdweb: no randomness source available"),
+        STDWEB_RNG_FAILED => Some("stdweb: failed to get randomness"),
+        _ => None,
     }
 }
 

src/error_impls.rs

@@ -7,28 +7,29 @@
 // except according to those terms.
 extern crate std;
 
-use crate::error::Error;
+use crate::{error::UNKNOWN_IO_ERROR, Error};
 use core::convert::From;
 use core::num::NonZeroU32;
-use std::{error, io};
+use std::io;
 
 impl From<io::Error> for Error {
     fn from(err: io::Error) -> Self {
-        err.raw_os_error()
-            .and_then(|code| NonZeroU32::new(code as u32))
-            .map(|code| Error(code))
-            // in practice this should never happen
-            .unwrap_or(Error::UNKNOWN)
+        if let Some(errno) = err.raw_os_error() {
+            if let Some(code) = NonZeroU32::new(errno as u32) {
+                return Error::from(code);
+            }
+        }
+        UNKNOWN_IO_ERROR
     }
 }
 
 impl From<Error> for io::Error {
     fn from(err: Error) -> Self {
-        match err.msg() {
-            Some(msg) => io::Error::new(io::ErrorKind::Other, msg),
-            None => io::Error::from_raw_os_error(err.0.get() as i32),
+        match err.raw_os_error() {
+            Some(errno) => io::Error::from_raw_os_error(errno),
+            None => io::Error::new(io::ErrorKind::Other, err),
         }
     }
 }
 
-impl error::Error for Error {}
+impl std::error::Error for Error {}

src/freebsd.rs

@@ -9,7 +9,6 @@
 //! Implementation for FreeBSD
 use crate::util_libc::{sys_fill_exact, Weak};
 use crate::Error;
-use core::num::NonZeroU32;
 use core::{mem, ptr};
 
 type GetRandomFn = unsafe extern "C" fn(*mut u8, libc::size_t, libc::c_uint) -> libc::ssize_t;
@@ -44,8 +43,3 @@ pub fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> {
         sys_fill_exact(dest, kern_arnd)
     }
 }
-
-#[inline(always)]
-pub fn error_msg_inner(_: NonZeroU32) -> Option<&'static str> {
-    None
-}

src/fuchsia.rs

@@ -8,7 +8,6 @@
 
 //! Implementation for Fuchsia Zircon
 use crate::Error;
-use core::num::NonZeroU32;
 
 #[link(name = "zircon")]
 extern "C" {
@@ -19,8 +18,3 @@ pub fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> {
     unsafe { zx_cprng_draw(dest.as_mut_ptr(), dest.len()) }
     Ok(())
 }
-
-#[inline(always)]
-pub fn error_msg_inner(_: NonZeroU32) -> Option<&'static str> {
-    None
-}

src/ios.rs

@@ -7,10 +7,7 @@
 // except according to those terms.
 
 //! Implementation for iOS
-extern crate std;
-
-use crate::Error;
-use core::num::NonZeroU32;
+use crate::{error::SEC_RANDOM_FAILED, Error};
 
 // TODO: Make extern once extern_types feature is stabilized. See:
 //   https://github.com/rust-lang/rust/issues/43467
@@ -27,14 +24,8 @@ extern "C" {
 pub fn getrandom_inner(dest: &mut [u8]) -> Result<(), Error> {
     let ret = unsafe { SecRandomCopyBytes(kSecRandomDefault, dest.len(), dest.as_mut_ptr()) };
     if ret == -1 {
-        error!("SecRandomCopyBytes call failed");
-        Err(Error::UNKNOWN)
+        Err(SEC_RANDOM_FAILED)
     } else {
         Ok(())
     }
 }
-
-#[inline(always)]
-pub fn error_msg_inner(_: NonZeroU32) -> Option<&'static str> {
-    None
-}

src/lib.rs

@@ -79,25 +79,8 @@
 //! `getrandom`, hence after the first successful call one can be reasonably
 //! confident that no errors will occur.
 //!
-//! On unsupported platforms, `getrandom` always fails with [`Error::UNAVAILABLE`].
-//!
-//! ## Error codes
-//! The crate uses the following custom error codes:
-//! - `0x57f4c500` (dec: 1475659008) - an unknown error. Constant:
-//! [`Error::UNKNOWN`]
-//! - `0x57f4c501` (dec: 1475659009) - no generator is available. Constant:
-//! [`Error::UNAVAILABLE`]
-//! - `0x57f4c580` (dec: 1475659136) - `self.crypto` is undefined,
-//! `wasm-bindgen` specific error.
-//! - `0x57f4c581` (dec: 1475659137) - `crypto.getRandomValues` is undefined,
-//! `wasm-bindgen` specific error.
-//!
-//! These codes are provided for reference only and should not be matched upon
-//! (but you can match on `Error` constants). The codes may change in future and
-//! such change will not be considered a breaking one.
-//!
-//! Other error codes will originate from an underlying system. In case if such
-//! error is encountered, please consult with your system documentation.
+//! On unsupported platforms, `getrandom` always fails. See the [`Error`] type
+//! for more information on what data is returned on failure.
 //!
 //! [1]: http://man7.org/linux/man-pages/man2/getrandom.2.html
 //! [2]: http://man7.org/linux/man-pages/man4/urandom.4.html
@@ -146,14 +129,12 @@ cfg_if! {
     }
 }
 
-#[cfg(feature = "std")]
-extern crate std;
-
 mod error;
 pub use crate::error::Error;
 
 #[allow(dead_code)]
 mod util;
+// Unlike the other Unix, Fuchsia and iOS don't use the libc to make any calls.
 #[cfg(any(
     target_os = "android",
     target_os = "dragonfly",
@@ -197,7 +178,6 @@ mod error_impls;
     target_os = "solaris",
     target_os = "illumos",
 ))]
-#[allow(dead_code)]
 mod use_file;
 
 // System-specific implementations.