[WIP] Define better errors with thiserror

zeroconf/Cargo.toml

@@ -26,6 +26,7 @@ derive-new = "0.5.9"
 log = "0.4.20"
 libc = "0.2.148"
 zeroconf-macros = { path = "../zeroconf-macros", version = "0.1.4" }
+thiserror = "1.0.63"
 
 [dev-dependencies]
 env_logger = "0.10.0"

zeroconf/src/bonjour/bonjour_util.rs

@@ -3,7 +3,9 @@
 use std::{ffi::CString, str::FromStr};
 
 use super::constants;
-use crate::{check_valid_characters, lstrip_underscore, NetworkInterface, Result, ServiceType};
+use crate::{
+    check_valid_characters, lstrip_underscore, Error, NetworkInterface, Result, ServiceType,
+};
 use bonjour_sys::DNSServiceErrorType;
 
 /// Normalizes the specified domain `&str` to conform to a standard enforced by this crate.
@@ -46,7 +48,10 @@ pub fn sys_exec<F: FnOnce() -> DNSServiceErrorType>(func: F, message: &str) -> R
     let err = func();
 
     if err < 0 {
-        Err(format!("{} (code: {})", message, err).into())
+        Err(Error::MdnsSystemError {
+            code: err,
+            message: message.into(),
+        })
     } else {
         Ok(())
     }
@@ -106,10 +111,9 @@ mod tests {
 
     #[test]
     fn parse_regtype_failure_invalid_regtype() {
-        assert_eq!(
-            parse_regtype("foobar"),
-            Err("invalid name and protocol".into())
-        );
+        let expected = Error::InvalidServiceType("invalid name and protocol".into());
+        let result = parse_regtype("foobar");
+        assert_eq!(result, Err(expected));
     }
 
     #[test]
@@ -132,10 +136,9 @@ mod tests {
 
     #[test]
     fn sys_exec_returns_error() {
-        assert_eq!(
-            sys_exec(|| -42, "uh oh spaghetti-o"),
-            Err("uh oh spaghetti-o (code: -42)".into())
-        );
+        let expected = Error::InvalidServiceType("uh oh spaghetti-o (code: -42)".into());
+        let result = sys_exec(|| -42, "uh oh spaghetti-o");
+        assert_eq!(result, Err(expected));
     }
 
     #[test]

zeroconf/src/bonjour/browser.rs

@@ -6,7 +6,7 @@ use super::service_ref::{
 use super::txt_record_ref::ManagedTXTRecordRef;
 use super::{bonjour_util, constants};
 use crate::ffi::{c_str, AsRaw, FromRaw};
-use crate::prelude::*;
+use crate::{prelude::*, Error};
 use crate::{EventLoop, NetworkInterface, Result, ServiceType, TxtRecord};
 use crate::{ServiceDiscoveredCallback, ServiceDiscovery};
 #[cfg(target_vendor = "pc")]
@@ -78,7 +78,8 @@ impl TMdnsBrowser for BonjourMdnsBrowser {
             .domain(ptr::null_mut())
             .callback(Some(browse_callback))
             .context(self.context.as_raw())
-            .build()?;
+            .build()
+            .map_err(Error::BrowserError)?;
 
         unsafe { service_lock.browse_services(browse_params)? };
 
@@ -143,7 +144,10 @@ unsafe fn handle_browse(
     interface_index: u32,
 ) -> Result<()> {
     if error != 0 {
-        return Err(format!("browse_callback() reported error (code: {})", error).into());
+        return Err(Error::MdnsSystemError {
+            code: error,
+            message: "browse_callback() reported error".into(),
+        });
     }
 
     ctx.resolved_name = Some(c_str::copy_raw(name));
@@ -159,7 +163,8 @@ unsafe fn handle_browse(
             .domain(domain)
             .callback(Some(resolve_callback))
             .context(ctx.as_raw())
-            .build()?,
+            .build()
+            .map_err(Error::BrowserError)?,
     )
 }
 
@@ -202,7 +207,10 @@ unsafe fn handle_resolve(
     txt_record: *const c_uchar,
 ) -> Result<()> {
     if error != 0 {
-        return Err(format!("error reported by resolve_callback: (code: {})", error).into());
+        return Err(Error::MdnsSystemError {
+            code: error,
+            message: "resolve_callback() reported error".into(),
+        });
     }
 
     ctx.resolved_port = port;
@@ -223,7 +231,8 @@ unsafe fn handle_resolve(
             .hostname(host_target)
             .callback(Some(get_address_info_callback))
             .context(ctx.as_raw())
-            .build()?,
+            .build()
+            .map_err(Error::BrowserError)?,
     )
 }
 
@@ -255,11 +264,10 @@ unsafe fn handle_get_address_info(
     }
 
     if error != 0 {
-        return Err(format!(
-            "get_address_info_callback() reported error (code: {})",
-            error
-        )
-        .into());
+        return Err(Error::MdnsSystemError {
+            code: error,
+            message: "get_address_info_callback() reported error".into(),
+        });
     }
 
     // on macOS the bytes are swapped for the port
@@ -285,22 +293,17 @@ unsafe fn handle_get_address_info(
 
     let hostname = c_str::copy_raw(hostname);
 
-    let domain = bonjour_util::normalize_domain(
-        &ctx.resolved_domain
-            .take()
-            .ok_or("could not get domain from BonjourBrowserContext")?,
-    );
+    let domain = bonjour_util::normalize_domain(&ctx.resolved_domain.take().ok_or(
+        Error::BrowserError("could not get domain from BonjourBrowserContext".into()),
+    )?);
 
-    let kind = bonjour_util::normalize_domain(
-        &ctx.resolved_kind
-            .take()
-            .ok_or("could not get kind from BonjourBrowserContext")?,
-    );
+    let kind = bonjour_util::normalize_domain(&ctx.resolved_kind.take().ok_or(
+        Error::BrowserError("could not get kind from BonjourBrowserContext".into()),
+    )?);
 
-    let name = ctx
-        .resolved_name
-        .take()
-        .ok_or("could not get name from BonjourBrowserContext")?;
+    let name = ctx.resolved_name.take().ok_or(Error::BrowserError(
+        "could not get name from BonjourBrowserContext".into(),
+    ))?;
 
     let result = ServiceDiscovery::builder()
         .name(name)

zeroconf/src/bonjour/service.rs

@@ -4,7 +4,7 @@ use super::service_ref::{ManagedDNSServiceRef, RegisterServiceParams};
 use super::{bonjour_util, constants};
 use crate::ffi::c_str::{self, AsCChars};
 use crate::ffi::{AsRaw, FromRaw, UnwrapOrNull};
-use crate::prelude::*;
+use crate::{prelude::*, Error};
 use crate::{
     EventLoop, NetworkInterface, Result, ServiceRegisteredCallback, ServiceRegistration,
     ServiceType, TxtRecord,
@@ -129,7 +129,8 @@ impl TMdnsService for BonjourMdnsService {
             .txt_record(txt_record)
             .callback(Some(register_callback))
             .context(self.context.as_raw())
-            .build()?;
+            .build()
+            .map_err(Error::ServiceError)?;
 
         unsafe { service_lock.register_service(register_params)? };
 
@@ -185,7 +186,10 @@ unsafe fn handle_register(
     regtype: *const c_char,
 ) -> Result<()> {
     if error != 0 {
-        return Err(format!("register_callback() reported error (code: {0})", error).into());
+        return Err(Error::MdnsSystemError {
+            code: error,
+            message: "register_callback() reported error".to_string(),
+        });
     }
 
     let domain = bonjour_util::normalize_domain(c_str::raw_to_str(domain));

zeroconf/src/error.rs

@@ -1,29 +1,67 @@
 //! Utilities regarding error handling
 
-use std::fmt;
+use thiserror::Error;
 
-/// For when something goes wrong when interfacing with mDNS implementations
-#[derive(new, Debug, Clone, PartialEq, Eq)]
-pub struct Error {
-    description: String,
+/// Error type for the zeroconf crate
+#[derive(Error, Debug, PartialEq)]
+pub enum Error {
+    /// An instance of `crate::ServiceType` could not be created due to an invalid format
+    #[error("Invalid ServiceType format: {0}")]
+    InvalidServiceType(String),
+    /// An error occurred in the underlying mDNS system (Avahi/Bonjour)
+    #[error("{message} (code: {code})")]
+    MdnsSystemError { code: i32, message: String },
+    /// An error occurred in the underlying system (ABI)
+    #[error("{message} (code: {code})")]
+    SystemError { code: i32, message: String },
+    /// An error occurred in an instance of an `crate::MdnsBrowser`
+    #[error("{0}")]
+    BrowserError(String),
+    /// An error occurred in an instance of an `crate::MdnsService`
+    #[error("{0}")]
+    ServiceError(String),
 }
 
-impl std::error::Error for Error {}
+#[cfg(test)]
+mod tests {
+    use super::*;
 
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.description)
+    #[test]
+    fn test_invalid_service_type_display() {
+        let error = Error::InvalidServiceType("invalid name and protocol".into());
+        assert_eq!(
+            error.to_string(),
+            "Invalid ServiceType format: invalid name and protocol"
+        );
     }
-}
 
-impl From<&str> for Error {
-    fn from(s: &str) -> Self {
-        Error::from(s.to_string())
+    #[test]
+    fn test_mdns_system_error_display() {
+        let error = Error::MdnsSystemError {
+            code: -42,
+            message: "uh oh spaghetti-o".into(),
+        };
+        assert_eq!(error.to_string(), "uh oh spaghetti-o (code: -42)");
+    }
+
+    #[test]
+    fn test_system_error_display() {
+        let error = Error::SystemError {
+            code: -42,
+            message: "uh oh spaghetti-o".into(),
+        };
+        assert_eq!(error.to_string(), "uh oh spaghetti-o (code: -42)");
+    }
+
+    #[test]
+    fn test_browser_error_display() {
+        let error = Error::BrowserError("uh oh spaghetti-o".into());
+        assert_eq!(error.to_string(), "uh oh spaghetti-o");
     }
-}
 
-impl From<String> for Error {
-    fn from(s: String) -> Self {
-        Error::new(s)
+    #[test]
+    fn test_service_error_display() {
+        let error = Error::ServiceError("uh oh spaghetti-o".into());
+        assert_eq!(error.to_string(), "uh oh spaghetti-o");
     }
 }

zeroconf/src/ffi/mod.rs

@@ -53,7 +53,7 @@ impl<T> UnwrapMutOrNull<T> for Option<*mut T> {
 
 #[cfg(target_vendor = "apple")]
 pub(crate) mod bonjour {
-    use crate::Result;
+    use crate::{Error, Result};
     use libc::{fd_set, suseconds_t, time_t, timeval};
     use std::time::Duration;
     use std::{mem, ptr};
@@ -82,7 +82,10 @@ pub(crate) mod bonjour {
         );
 
         if result < 0 {
-            Err("select(): returned error status".into())
+            Err(Error::SystemError {
+                code: result,
+                message: "select(): returned error status".to_string(),
+            })
         } else {
             Ok(result as u32)
         }

zeroconf/src/lib.rs

@@ -229,6 +229,7 @@ pub mod avahi;
 pub mod bonjour;
 
 pub use browser::{ServiceDiscoveredCallback, ServiceDiscovery};
+pub use error::Error;
 pub use interface::*;
 pub use service::{ServiceRegisteredCallback, ServiceRegistration};
 pub use service_type::*;

zeroconf/src/service_type.rs

@@ -44,7 +44,8 @@ impl FromStr for ServiceType {
         let parts = s.split('.').collect::<Vec<_>>();
 
         if parts.len() != 2 {
-            return Err("invalid name and protocol".into());
+            let msg = "invalid name and protocol";
+            return Err(Error::InvalidServiceType(msg.into()));
         }
 
         let name = lstrip_underscore(check_valid_characters(parts[0])?);
@@ -56,11 +57,12 @@ impl FromStr for ServiceType {
 
 pub fn check_valid_characters(part: &str) -> Result<&str> {
     if part.contains('.') {
-        Err("invalid character: .".into())
+        let msg = "invalid character: .";
+        Err(Error::InvalidServiceType(msg.into()))
     } else if part.contains(',') {
-        Err("invalid character: ,".into())
+        Err(Error::InvalidServiceType("invalid character: ,".into()))
     } else if part.is_empty() {
-        Err("cannot be empty".into())
+        Err(Error::InvalidServiceType("cannot be empty".into()))
     } else {
         Ok(part)
     }