Error descriptions and docs

boring/src/error.rs

@@ -15,7 +15,8 @@
 //!     Err(e) => println!("Parsing Error: {:?}", e),
 //! }
 //! ```
-use libc::{c_char, c_uint};
+use libc::{c_char, c_int, c_uint};
+use openssl_macros::corresponds;
 use std::borrow::Cow;
 use std::error;
 use std::ffi::CStr;
@@ -80,7 +81,9 @@ impl fmt::Display for ErrorStack {
             write!(
                 fmt,
                 "[{}]",
-                err.reason_internal().unwrap_or("unknown reason")
+                err.reason_internal()
+                    .or_else(|| err.library())
+                    .unwrap_or("unknown reason")
             )?;
         }
         Ok(())
@@ -101,7 +104,7 @@ impl From<ErrorStack> for fmt::Error {
     }
 }
 
-/// An error reported from OpenSSL.
+/// A detailed error reported as part of an [`ErrorStack`].
 #[derive(Clone)]
 pub struct Error {
     code: c_uint,
@@ -179,7 +182,10 @@ impl Error {
         }
     }
 
-    /// Returns the raw OpenSSL error code for this error.
+    /// Returns a raw OpenSSL **packed** error code for this error, which **can't be reliably compared to any error constant**.
+    ///
+    /// Use [`Error::library_code()`] and [`Error::reason_code()`] instead.
+    /// Packed error codes are different than [SSL error codes](crate::ssl::ErrorCode).
     #[must_use]
     pub fn code(&self) -> c_uint {
         self.code
@@ -201,10 +207,11 @@ impl Error {
         }
     }
 
-    /// Returns the raw OpenSSL error constant for the library reporting the
+    /// Returns the raw OpenSSL error constant for the library reporting the error (`ERR_LIB_{name}`).
-    /// error.
+    ///
+    /// Error [reason codes](Error::reason_code) are not globally unique, but scoped to each library.
     #[must_use]
-    pub fn library_code(&self) -> libc::c_int {
+    pub fn library_code(&self) -> c_int {
         ffi::ERR_GET_LIB(self.code)
     }
 
@@ -226,9 +233,14 @@ impl Error {
         }
     }
 
-    /// Returns the raw OpenSSL error constant for the reason for the error.
+    /// Returns [library-specific](Error::library_code) reason code corresponding to some of the `{lib}_R_{reason}` constants.
+    ///
+    /// Reason codes are ambiguous, and different libraries reuse the same numeric values for different errors.
+    ///
+    /// For `ERR_LIB_SYS` the reason code is `errno`. `ERR_LIB_USER` can use any values.
+    /// Other libraries may use [`ERR_R_*`](ffi::ERR_R_FATAL) or their own codes.
     #[must_use]
-    pub fn reason_code(&self) -> libc::c_int {
+    pub fn reason_code(&self) -> c_int {
         ffi::ERR_GET_REASON(self.code)
     }
 
@@ -285,17 +297,19 @@ impl Error {
 impl fmt::Debug for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let mut builder = fmt.debug_struct("Error");
-        builder.field("code", &self.code());
+        builder.field("code", &self.code);
-        if let Some(library) = self.library() {
+        if !self.is_internal() {
-            builder.field("library", &library);
+            if let Some(library) = self.library() {
+                builder.field("library", &library);
+            }
+            builder.field("library_code", &self.library_code());
+            if let Some(reason) = self.reason() {
+                builder.field("reason", &reason);
+            }
+            builder.field("reason_code", &self.reason_code());
+            builder.field("file", &self.file());
+            builder.field("line", &self.line());
         }
-        builder.field("library_code", &self.library_code());
-        if let Some(reason) = self.reason() {
-            builder.field("reason", &reason);
-        }
-        builder.field("reason_code", &self.reason_code());
-        builder.field("file", &self.file());
-        builder.field("line", &self.line());
         if let Some(data) = self.data() {
             builder.field("data", &data);
         }
@@ -309,7 +323,7 @@ impl fmt::Display for Error {
             fmt,
             "{}\n\nCode: {:08X}\nLoc: {}:{}",
             self.reason_internal().unwrap_or("unknown TLS error"),
-            self.code(),
+            &self.code,
             self.file(),
             self.line()
         )

boring/src/ssl/error.rs

@@ -1,16 +1,20 @@
 use crate::ffi;
 use crate::x509::X509VerifyError;
 use libc::c_int;
+use openssl_macros::corresponds;
 use std::error;
 use std::error::Error as StdError;
+use std::ffi::CStr;
 use std::fmt;
 use std::io;
 
 use crate::error::ErrorStack;
 use crate::ssl::MidHandshakeSslStream;
 
-/// An error code returned from SSL functions.
+/// `SSL_ERROR_*` error code returned from SSL functions.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+///
+/// This is different than [packed error codes](crate::error::Error).
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub struct ErrorCode(c_int);
 
 impl ErrorCode {
@@ -50,16 +54,52 @@ impl ErrorCode {
     /// An error occurred in the SSL library.
     pub const SSL: ErrorCode = ErrorCode(ffi::SSL_ERROR_SSL);
 
+    /// Wrap an `SSL_ERROR_*` error code.
+    ///
+    /// This is different than [packed error codes](crate::error::Error).
     #[must_use]
+    #[inline]
+    #[cfg_attr(debug_assertions, track_caller)]
     pub fn from_raw(raw: c_int) -> ErrorCode {
-        ErrorCode(raw)
+        let code = ErrorCode(raw);
+        debug_assert!(
+            raw < 64 || code.description().is_some(),
+            "{raw} is not an SSL_ERROR_* code"
+        );
+        code
     }
 
+    /// An `SSL_ERROR_*` error code.
+    ///
+    /// This is different than [packed error codes](crate::error::Error).
     #[allow(clippy::trivially_copy_pass_by_ref)]
     #[must_use]
     pub fn as_raw(&self) -> c_int {
         self.0
     }
+
+    #[corresponds(SSL_error_description)]
+    pub fn description(self) -> Option<&'static str> {
+        unsafe {
+            let msg = ffi::SSL_error_description(self.0);
+            if msg.is_null() {
+                return None;
+            }
+            CStr::from_ptr(msg).to_str().ok()
+        }
+    }
+}
+
+impl fmt::Display for ErrorCode {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{} ({})", self.description().unwrap_or("error"), self.0)
+    }
+}
+
+impl fmt::Debug for ErrorCode {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Display::fmt(self, f)
+    }
 }
 
 #[derive(Debug)]
@@ -68,7 +108,7 @@ pub(crate) enum InnerError {
     Ssl(ErrorStack),
 }
 
-/// An SSL error.
+/// A general SSL error, based on [`SSL_ERROR_*` error codes](ErrorCode).
 #[derive(Debug)]
 pub struct Error {
     pub(crate) code: ErrorCode,
@@ -76,6 +116,7 @@ pub struct Error {
 }
 
 impl Error {
+    /// An `SSL_ERROR_*` error code.
     #[must_use]
     pub fn code(&self) -> ErrorCode {
         self.code
@@ -96,6 +137,7 @@ impl Error {
         }
     }
 
+    /// Stack of [library-specific errors](crate::error::Error), if available.
     #[must_use]
     pub fn ssl_error(&self) -> Option<&ErrorStack> {
         match self.cause {
@@ -131,26 +173,27 @@ impl From<ErrorStack> for Error {
 
 impl fmt::Display for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        match self.code {
+        let msg = match self.code {
-            ErrorCode::ZERO_RETURN => fmt.write_str("the SSL session has been shut down"),
+            ErrorCode::ZERO_RETURN => "the SSL session has been shut down",
             ErrorCode::WANT_READ => match self.io_error() {
-                Some(_) => fmt.write_str("a nonblocking read call would have blocked"),
+                Some(_) => "a nonblocking read call would have blocked",
-                None => fmt.write_str("the operation should be retried"),
+                None => "the operation should be retried",
             },
             ErrorCode::WANT_WRITE => match self.io_error() {
-                Some(_) => fmt.write_str("a nonblocking write call would have blocked"),
+                Some(_) => "a nonblocking write call would have blocked",
-                None => fmt.write_str("the operation should be retried"),
+                None => "the operation should be retried",
             },
             ErrorCode::SYSCALL => match self.io_error() {
-                Some(err) => write!(fmt, "{err}"),
+                Some(err) => return err.fmt(fmt),
-                None => fmt.write_str("unexpected EOF"),
+                None => "unexpected EOF",
             },
             ErrorCode::SSL => match self.ssl_error() {
-                Some(e) => write!(fmt, "{e}"),
+                Some(err) => return err.fmt(fmt),
-                None => fmt.write_str("unknown BoringSSL error"),
+                None => "unknown BoringSSL error",
             },
-            ErrorCode(code) => write!(fmt, "unknown error code {code}"),
+            ErrorCode(code) => return code.fmt(fmt),
-        }
+        };
+        fmt.write_str(msg)
     }
 }