From bd7f941c58cecd14dc2f66050a173dd22e1ec2f7 Mon Sep 17 00:00:00 2001 From: Peter Jankuliak Date: Mon, 27 Jan 2025 11:49:01 +0100 Subject: [PATCH 1/2] define better errors with thiserror for Avahi as well --- examples/browser/src/main.rs | 7 ++--- zeroconf/src/avahi/avahi_util.rs | 22 ++++++++++---- zeroconf/src/avahi/browser.rs | 49 ++++++++++++++++++------------- zeroconf/src/avahi/client.rs | 21 ++++++------- zeroconf/src/avahi/entry_group.rs | 11 ++++--- zeroconf/src/avahi/poll.rs | 20 +++++++------ zeroconf/src/avahi/raw_browser.rs | 4 +-- zeroconf/src/avahi/resolver.rs | 4 +-- zeroconf/src/avahi/service.rs | 46 ++++++++++++++++------------- 9 files changed, 102 insertions(+), 82 deletions(-) diff --git a/examples/browser/src/main.rs b/examples/browser/src/main.rs index 0797c83..1c1c981 100644 --- a/examples/browser/src/main.rs +++ b/examples/browser/src/main.rs @@ -51,7 +51,7 @@ fn main() -> zeroconf::Result<()> { loop { // calling `poll()` will keep this browser alive - event_loop.poll(Duration::from_secs(0))?; + event_loop.poll(Duration::from_secs(1))?; } } @@ -59,10 +59,7 @@ fn on_service_discovered( result: zeroconf::Result, _context: Option>, ) { - info!( - "Service discovered: {:?}", - result.expect("service discovery failed") - ); + info!("Service discovery event: {:?}", result); // ... } diff --git a/zeroconf/src/avahi/avahi_util.rs b/zeroconf/src/avahi/avahi_util.rs index a683cd4..925d193 100644 --- a/zeroconf/src/avahi/avahi_util.rs +++ b/zeroconf/src/avahi/avahi_util.rs @@ -1,6 +1,6 @@ //! Utilities related to Avahi -use crate::ffi::c_str; +use crate::{ffi::c_str, Error}; use avahi_sys::{ avahi_address_snprint, avahi_alternative_service_name, avahi_strerror, AvahiAddress, AvahiClient, @@ -47,8 +47,14 @@ pub unsafe fn get_error<'a>(code: i32) -> &'a str { /// /// # Safety /// This function is unsafe because of internal Avahi calls. -pub unsafe fn get_last_error<'a>(client: *mut AvahiClient) -> &'a str { - get_error(avahi_sys::avahi_client_errno(client)) +pub unsafe fn get_last_error(client: *mut AvahiClient) -> Error { + let code = avahi_sys::avahi_client_errno(client); + let message = get_error(code); + + Error::MdnsSystemError { + code, + message: message.into(), + } } /// Converts the specified [`NetworkInterface`] to the Avahi expected value. @@ -77,7 +83,10 @@ pub unsafe fn sys_exec i32>(func: F, message: &str) -> Result<()> let err = func(); if err < 0 { - Err(format!("{}: `{}`", message, get_error(err)).into()) + Err(Error::MdnsSystemError { + code: err, + message: format!("{}: (code: {}, message:{:?})", message, err, get_error(err)).into(), + }) } else { Ok(()) } @@ -139,7 +148,10 @@ mod tests { fn sys_exec_returns_error_for_failure() { assert_eq!( unsafe { sys_exec(|| avahi_sys::AVAHI_ERR_FAILURE, "uh oh spaghetti-o") }, - Err("uh oh spaghetti-o: `Operation failed`".into()) + Err(Error::MdnsSystemError { + code: avahi_sys::AVAHI_ERR_FAILURE, + message: "uh oh spaghetti-o: (code: -1, message:\"Operation failed\")".into(), + }) ); } diff --git a/zeroconf/src/avahi/browser.rs b/zeroconf/src/avahi/browser.rs index f041178..2904db4 100644 --- a/zeroconf/src/avahi/browser.rs +++ b/zeroconf/src/avahi/browser.rs @@ -12,7 +12,7 @@ use super::{ }; use crate::ffi::{c_str, AsRaw, FromRaw}; use crate::prelude::*; -use crate::Result; +use crate::{Error, Result}; use crate::{ EventLoop, NetworkInterface, ServiceDiscoveredCallback, ServiceDiscovery, ServiceType, TxtRecord, @@ -74,12 +74,16 @@ impl TMdnsBrowser for AvahiMdnsBrowser { fn browse_services(&mut self) -> Result { debug!("Browsing services: {:?}", self); - self.poll = Some(Arc::new(unsafe { ManagedAvahiSimplePoll::new() }?)); + self.poll = Some(Arc::new( + unsafe { ManagedAvahiSimplePoll::new() }.ok_or_else(|| { + Error::BrowserError("could not initialize AvahiSimplePoll".into()) + })?, + )); let poll = self .poll .as_ref() - .ok_or("could not get poll as ref")? + .ok_or(Error::BrowserError("could not get poll as ref".into()))? .clone(); let client_params = ManagedAvahiClientParams::builder() @@ -87,7 +91,8 @@ impl TMdnsBrowser for AvahiMdnsBrowser { .flags(AvahiClientFlags(0)) .callback(Some(client_callback)) .userdata(self.context.as_raw()) - .build()?; + .build() + .map_err(Error::BrowserError)?; self.client = Some(Arc::new(unsafe { ManagedAvahiClient::new(client_params) }?)); @@ -102,7 +107,7 @@ impl TMdnsBrowser for AvahiMdnsBrowser { Ok(EventLoop::new( self.poll .as_ref() - .ok_or("could not get poll as ref")? + .ok_or(Error::BrowserError("could not get poll as ref".into()))? .clone(), )) } @@ -171,13 +176,11 @@ unsafe fn create_browser(context: &mut AvahiBrowserContext) -> Result<()> { .flags(0) .callback(Some(browse_callback)) .userdata(context.as_raw()) - .client(Arc::clone( - context - .client - .as_ref() - .ok_or("could not get client as ref")?, - )) - .build()?, + .client(Arc::clone(context.client.as_ref().ok_or( + Error::BrowserError("could not get client as ref".into()), + )?)) + .build() + .map_err(Error::BrowserError)?, )?); Ok(()) @@ -203,7 +206,7 @@ unsafe extern "C" fn browse_callback( } } avahi_sys::AvahiBrowserEvent_AVAHI_BROWSER_FAILURE => { - context.invoke_callback(Err("browser failure".into())) + context.invoke_callback(Err(Error::BrowserError("browser failure".into()))) } _ => {} }; @@ -222,7 +225,7 @@ unsafe fn handle_browser_new( let client = context .client .as_ref() - .ok_or("expected initialized client")?; + .ok_or(Error::BrowserError("expected initialized client".into()))?; context.resolvers.insert(ManagedAvahiServiceResolver::new( ManagedAvahiServiceResolverParams::builder() @@ -236,7 +239,8 @@ unsafe fn handle_browser_new( .flags(0) .callback(Some(resolve_callback)) .userdata(raw_context) - .build()?, + .build() + .map_err(Error::BrowserError)?, )?); Ok(()) @@ -265,11 +269,13 @@ unsafe extern "C" fn resolve_callback( match event { avahi_sys::AvahiResolverEvent_AVAHI_RESOLVER_FAILURE => { - context.invoke_callback(Err(format!( - "failed to resolve service `{}` of type `{}` in domain `{}`", - name, kind, domain - ) - .into())); + context.invoke_callback(Err(Error::BrowserError( + format!( + "failed to resolve service `{}` of type `{}` in domain `{}`", + name, kind, domain + ) + .into(), + ))); } avahi_sys::AvahiResolverEvent_AVAHI_RESOLVER_FOUND => { let result = handle_resolver_found( @@ -320,7 +326,8 @@ unsafe fn handle_resolver_found( .address(address) .port(port) .txt(txt) - .build()?; + .build() + .map_err(Error::BrowserError)?; debug!("Service resolved: {:?}", result); diff --git a/zeroconf/src/avahi/client.rs b/zeroconf/src/avahi/client.rs index ecc6075..6cec903 100644 --- a/zeroconf/src/avahi/client.rs +++ b/zeroconf/src/avahi/client.rs @@ -2,9 +2,10 @@ use std::sync::Arc; -use super::{avahi_util, poll::ManagedAvahiSimplePoll}; +use super::avahi_util; +use super::poll::ManagedAvahiSimplePoll; use crate::ffi::c_str; -use crate::Result; +use crate::{Error, Result}; use avahi_sys::{ avahi_client_free, avahi_client_get_host_name, avahi_client_new, avahi_simple_poll_get, AvahiClient, AvahiClientCallback, AvahiClientFlags, @@ -46,17 +47,13 @@ impl ManagedAvahiClient { ); if inner.is_null() { - return Err("could not initialize AvahiClient".into()); + return Err(Error::MdnsSystemError { + code: err, + message: avahi_util::get_error(err).into(), + }); } - match err { - 0 => Ok(Self { inner, _poll: poll }), - _ => Err(format!( - "could not initialize AvahiClient: {}", - avahi_util::get_error(err) - ) - .into()), - } + Ok(Self { inner, _poll: poll }) } /// Delegate function for [`avahi_client_get_host_name()`]. @@ -99,6 +96,6 @@ pub(super) unsafe fn get_host_name<'a>(client: *mut AvahiClient) -> Result<&'a s if !host_name.is_null() { Ok(c_str::raw_to_str(host_name)) } else { - Err("could not get host name from AvahiClient".into()) + Err(avahi_util::get_last_error(client)) } } diff --git a/zeroconf/src/avahi/entry_group.rs b/zeroconf/src/avahi/entry_group.rs index 9babd93..bd5ea3a 100644 --- a/zeroconf/src/avahi/entry_group.rs +++ b/zeroconf/src/avahi/entry_group.rs @@ -7,10 +7,10 @@ use crate::avahi::avahi_util; use crate::ffi::UnwrapMutOrNull; use crate::Result; use avahi_sys::{ - avahi_client_errno, avahi_entry_group_add_service_strlst, - avahi_entry_group_add_service_subtype, avahi_entry_group_commit, avahi_entry_group_free, - avahi_entry_group_is_empty, avahi_entry_group_new, avahi_entry_group_reset, AvahiClient, - AvahiEntryGroup, AvahiEntryGroupCallback, AvahiIfIndex, AvahiProtocol, AvahiPublishFlags, + avahi_entry_group_add_service_strlst, avahi_entry_group_add_service_subtype, + avahi_entry_group_commit, avahi_entry_group_free, avahi_entry_group_is_empty, + avahi_entry_group_new, avahi_entry_group_reset, AvahiClient, AvahiEntryGroup, + AvahiEntryGroupCallback, AvahiIfIndex, AvahiProtocol, AvahiPublishFlags, }; use libc::{c_char, c_void}; @@ -40,8 +40,7 @@ impl ManagedAvahiEntryGroup { let inner = avahi_entry_group_new(client.inner, callback, userdata); if inner.is_null() { - let err = avahi_util::get_error(avahi_client_errno(client.inner)); - Err(format!("could not initialize AvahiEntryGroup: {}", err).into()) + Err(avahi_util::get_last_error(client.inner)) } else { Ok(Self { inner, diff --git a/zeroconf/src/avahi/poll.rs b/zeroconf/src/avahi/poll.rs index 6a5096b..bbdb8df 100644 --- a/zeroconf/src/avahi/poll.rs +++ b/zeroconf/src/avahi/poll.rs @@ -21,12 +21,12 @@ impl ManagedAvahiSimplePoll { /// /// # Safety /// This function is unsafe because of the raw pointer dereference. - pub unsafe fn new() -> Result { + pub unsafe fn new() -> Option { let poll = avahi_simple_poll_new(); if poll.is_null() { - Err("could not initialize AvahiSimplePoll".into()) + None } else { - Ok(Self(poll)) + Some(Self(poll)) } } @@ -58,12 +58,14 @@ impl ManagedAvahiSimplePoll { // Returns -1 on error, 0 on success and 1 if a quit request has been scheduled match avahi_simple_poll_iterate(self.0, sleep_time) { 0 | 1 => Ok(()), - -1 => Err(Error::from( - "avahi_simple_poll_iterate(..) threw an error result", - )), - _ => Err(Error::from( - "avahi_simple_poll_iterate(..) returned an unknown result", - )), + -1 => Err(Error::MdnsSystemError { + code: -1, // Translates to AVAHI_ERR_FAILURE with description "Generic error code". + message: "avahi_simple_poll_iterate(..) threw an error result".into(), + }), + err => Err(Error::MdnsSystemError { + code: err, + message: "avahi_simple_poll_iterate(..) returned an unknown result".into(), + }), } } diff --git a/zeroconf/src/avahi/raw_browser.rs b/zeroconf/src/avahi/raw_browser.rs index 8124539..2e20a59 100644 --- a/zeroconf/src/avahi/raw_browser.rs +++ b/zeroconf/src/avahi/raw_browser.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use crate::Result; +use crate::{avahi::avahi_util, Result}; use avahi_sys::{ avahi_service_browser_free, avahi_service_browser_get_client, avahi_service_browser_new, AvahiClient, AvahiIfIndex, AvahiLookupFlags, AvahiProtocol, AvahiServiceBrowser, @@ -52,7 +52,7 @@ impl ManagedAvahiServiceBrowser { ); if inner.is_null() { - Err("could not initialize Avahi service browser".into()) + Err(avahi_util::get_last_error(client.inner)) } else { Ok(Self { inner, diff --git a/zeroconf/src/avahi/resolver.rs b/zeroconf/src/avahi/resolver.rs index 3ff11a3..fda9b33 100644 --- a/zeroconf/src/avahi/resolver.rs +++ b/zeroconf/src/avahi/resolver.rs @@ -1,6 +1,6 @@ //! Rust friendly `AvahiServiceResolver` wrappers/helpers -use crate::Result; +use crate::{avahi::avahi_util, Result}; use avahi_sys::{ avahi_service_resolver_free, avahi_service_resolver_new, AvahiIfIndex, AvahiLookupFlags, AvahiProtocol, AvahiServiceResolver, AvahiServiceResolverCallback, @@ -55,7 +55,7 @@ impl ManagedAvahiServiceResolver { ); if inner.is_null() { - Err("could not initialize AvahiServiceResolver".into()) + Err(avahi_util::get_last_error(client.inner)) } else { Ok(Self { inner, diff --git a/zeroconf/src/avahi/service.rs b/zeroconf/src/avahi/service.rs index 5e068bc..3e11124 100644 --- a/zeroconf/src/avahi/service.rs +++ b/zeroconf/src/avahi/service.rs @@ -9,7 +9,7 @@ use super::poll::ManagedAvahiSimplePoll; use crate::ffi::{c_str, AsRaw, FromRaw, UnwrapOrNull}; use crate::prelude::*; use crate::{ - EventLoop, NetworkInterface, Result, ServiceRegisteredCallback, ServiceRegistration, + Error, EventLoop, NetworkInterface, Result, ServiceRegisteredCallback, ServiceRegistration, ServiceType, TxtRecord, }; use avahi_sys::{ @@ -110,12 +110,16 @@ impl TMdnsService for AvahiMdnsService { fn register(&mut self) -> Result { debug!("Registering service: {:?}", self); - self.poll = Some(Arc::new(unsafe { ManagedAvahiSimplePoll::new() }?)); + self.poll = Some(Arc::new( + unsafe { ManagedAvahiSimplePoll::new() }.ok_or_else(|| { + Error::ServiceError("could not initialize AvahiSimplePoll".into()) + })?, + )); let poll = self .poll .as_ref() - .ok_or("could not get poll as ref")? + .ok_or(Error::ServiceError("could not get poll as ref".into()))? .clone(); let client_params = ManagedAvahiClientParams::builder() @@ -123,7 +127,8 @@ impl TMdnsService for AvahiMdnsService { .flags(AvahiClientFlags(0)) .callback(Some(client_callback)) .userdata(self.context.as_raw()) - .build()?; + .build() + .map_err(Error::ServiceError)?; self.client = Some(Arc::new(unsafe { ManagedAvahiClient::new(client_params) }?)); @@ -138,7 +143,7 @@ impl TMdnsService for AvahiMdnsService { Ok(EventLoop::new( self.poll .as_ref() - .ok_or("could not get poll as ref")? + .ok_or(Error::ServiceError("could not get poll as ref".into()))? .clone(), )) } @@ -220,7 +225,7 @@ unsafe fn create_service(context: &mut AvahiServiceContext) -> Result<()> { let host_name = context .client .as_ref() - .ok_or("expected initialized client")? + .ok_or(Error::ServiceError("expected initialized client".into()))? .host_name()?; context.name = Some(c_string!(host_name.to_string())); @@ -231,22 +236,20 @@ unsafe fn create_service(context: &mut AvahiServiceContext) -> Result<()> { context.group = Some(ManagedAvahiEntryGroup::new( ManagedAvahiEntryGroupParams::builder() - .client(Arc::clone( - context - .client - .as_ref() - .ok_or("could not get client as ref")?, - )) + .client(Arc::clone(context.client.as_ref().ok_or( + Error::ServiceError("could not get client as ref".into()), + )?)) .callback(Some(entry_group_callback)) .userdata(context.as_raw()) - .build()?, + .build() + .map_err(Error::ServiceError)?, )?); } let group = context .group .as_mut() - .ok_or("could not borrow group as mut")?; + .ok_or(Error::ServiceError("could not borrow group as mut".into()))?; if !group.is_empty() { return Ok(()); @@ -255,7 +258,7 @@ unsafe fn create_service(context: &mut AvahiServiceContext) -> Result<()> { let name = context .name .as_ref() - .ok_or("could not get name as ref")? + .ok_or(Error::ServiceError("could not get name as ref".into()))? .clone(); add_services(context, &name) @@ -267,7 +270,7 @@ unsafe fn add_services(context: &mut AvahiServiceContext, name: &CStr) -> Result let group = context .group .as_mut() - .ok_or("could not borrow group as mut")?; + .ok_or(Error::ServiceError("could not borrow group as mut".into()))?; let params = AddServiceParams::builder() .interface(context.interface_index) @@ -279,7 +282,8 @@ unsafe fn add_services(context: &mut AvahiServiceContext, name: &CStr) -> Result .host(context.host.as_ref().map(|h| h.as_ptr()).unwrap_or_null()) .port(context.port) .txt(context.txt_record.as_ref().map(|t| t.inner())) - .build()?; + .build() + .map_err(Error::ServiceError)?; group.add_service(params)?; @@ -294,7 +298,8 @@ unsafe fn add_services(context: &mut AvahiServiceContext, name: &CStr) -> Result .kind(context.kind.as_ptr()) .domain(context.domain.as_ref().map(|d| d.as_ptr()).unwrap_or_null()) .subtype(sub_type.as_ptr()) - .build()?; + .build() + .map_err(Error::ServiceError)?; group.add_service_subtype(params)?; } @@ -348,7 +353,7 @@ unsafe fn handle_group_established(context: &AvahiServiceContext) -> Result Result Date: Mon, 27 Jan 2025 12:10:30 +0100 Subject: [PATCH 2/2] Fix bonjour test --- zeroconf/src/bonjour/bonjour_util.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/zeroconf/src/bonjour/bonjour_util.rs b/zeroconf/src/bonjour/bonjour_util.rs index bab39b4..27ff898 100644 --- a/zeroconf/src/bonjour/bonjour_util.rs +++ b/zeroconf/src/bonjour/bonjour_util.rs @@ -50,7 +50,7 @@ pub fn sys_exec DNSServiceErrorType>(func: F, message: &str) -> R if err < 0 { Err(Error::MdnsSystemError { code: err, - message: message.into(), + message: format!("{} (code: {})", message, err).into(), }) } else { Ok(()) @@ -136,9 +136,11 @@ mod tests { #[test] fn sys_exec_returns_error() { - 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)); + assert_eq!(result, Err(Error::MdnsSystemError { + code: -42, + message: "uh oh spaghetti-o (code: -42)".into(), + })); } #[test]