🔥 message mod API improvements

book/src/server.md

@@ -66,10 +66,10 @@ let mut stream = zbus::MessageStream::from(&connection);
 #        .await?;
 #
 while let Some(msg) = stream.try_next().await? {
-    let msg_header = msg.header()?;
+    let msg_header = msg.header();
     dbg!(&msg);
 
-    match msg_header.message_type()? {
+    match msg_header.message_type() {
         zbus::message::Type::MethodCall => {
             // real code would check msg_header path(), interface() and member()
             // handle invalid calls, introspection, errors etc

zbus/src/connection/mod.rs

@@ -261,7 +261,7 @@ impl OrderedFuture for PendingMethodCall {
                         data: Ok(msg),
                         ordering,
                     }) => {
-                        if msg.reply_serial() != Some(this.serial) {
+                        if msg.header().reply_serial() != Some(this.serial) {
                             continue;
                         }
                         let res = match msg.message_type() {
@@ -996,18 +996,11 @@ impl Connection {
                         m.ok()
                     }) {
                         if let Some(conn) = weak_conn.upgrade() {
-                            let hdr = match msg.header() {
-                                Ok(hdr) => hdr,
-                                Err(e) => {
-                                    warn!("Failed to parse header: {}", e);
-
-                                    continue;
-                                }
-                            };
+                            let hdr = msg.header();
                             match hdr.destination() {
                                 // Unique name is already checked by the match rule.
-                                Ok(Some(BusName::Unique(_))) | Ok(None) => (),
-                                Ok(Some(BusName::WellKnown(dest))) => {
+                                Some(BusName::Unique(_)) | None => (),
+                                Some(BusName::WellKnown(dest)) => {
                                     let names = conn.inner.registered_names.lock().await;
                                     // destination doesn't matter if no name has been registered
                                     // (probably means name it's registered through external means).
@@ -1017,13 +1010,8 @@ impl Connection {
                                         continue;
                                     }
                                 }
-                                Err(e) => {
-                                    warn!("Failed to parse destination: {}", e);
-
-                                    continue;
-                                }
                             }
-                            let member = match msg.member() {
+                            let member = match hdr.member() {
                                 Some(member) => member,
                                 None => {
                                     warn!("Got a method call with no `MEMBER` field: {}", msg);

zbus/src/error.rs

@@ -52,7 +52,6 @@ pub enum Error {
     #[cfg(feature = "xml")]
     /// An XML error from quick_xml
     QuickXml(DeError),
-    NoBodySignature,
     /// The requested name was already claimed by another peer.
     NameTaken,
     /// Invalid [match rule][MR] string.
@@ -84,7 +83,6 @@ impl PartialEq for Error {
             (Self::InvalidSerial, Self::InvalidSerial) => true,
             (Self::Unsupported, Self::Unsupported) => true,
             (Self::FDO(s), Self::FDO(o)) => s == o,
-            (Self::NoBodySignature, Self::NoBodySignature) => true,
             (Self::InvalidField, Self::InvalidField) => true,
             (Self::InvalidMatchRule, Self::InvalidMatchRule) => true,
             (Self::Variant(s), Self::Variant(o)) => s == o,
@@ -117,7 +115,6 @@ impl error::Error for Error {
             Error::FDO(e) => Some(e),
             #[cfg(feature = "xml")]
             Error::QuickXml(e) => Some(e),
-            Error::NoBodySignature => None,
             Error::InvalidField => None,
             Error::MissingField => None,
             Error::NameTaken => None,
@@ -154,7 +151,6 @@ impl fmt::Display for Error {
             Error::FDO(e) => write!(f, "{e}"),
             #[cfg(feature = "xml")]
             Error::QuickXml(e) => write!(f, "XML error: {e}"),
-            Error::NoBodySignature => write!(f, "missing body signature in the message"),
             Error::NameTaken => write!(f, "name already taken on the bus"),
             Error::InvalidMatchRule => write!(f, "Invalid match rule string"),
             Error::Failure(e) => write!(f, "{e}"),
@@ -188,7 +184,6 @@ impl Clone for Error {
             Error::FDO(e) => Error::FDO(e.clone()),
             #[cfg(feature = "xml")]
             Error::QuickXml(e) => Error::QuickXml(e.clone()),
-            Error::NoBodySignature => Error::NoBodySignature,
             Error::NameTaken => Error::NameTaken,
             Error::InvalidMatchRule => Error::InvalidMatchRule,
             Error::Failure(e) => Error::Failure(e.clone()),
@@ -259,17 +254,12 @@ impl From<Arc<Message>> for Error {
     fn from(message: Arc<Message>) -> Error {
         // FIXME: Instead of checking this, we should have Method as trait and specific types for
         // each message type.
-        let header = match message.header() {
-            Ok(header) => header,
-            Err(e) => {
-                return e;
-            }
-        };
+        let header = message.header();
         if header.primary().msg_type() != Type::Error {
             return Error::InvalidReply;
         }
 
-        if let Ok(Some(name)) = header.error_name() {
+        if let Some(name) = header.error_name() {
             let name = name.to_owned().into();
             match message.body_unchecked::<&str>() {
                 Ok(detail) => Error::MethodError(name, Some(String::from(detail)), message),

zbus/src/fdo.rs

@@ -54,7 +54,7 @@ impl Introspectable {
         #[zbus(object_server)] server: &ObjectServer,
         #[zbus(header)] header: Header<'_>,
     ) -> Result<String> {
-        let path = header.path()?.ok_or(crate::Error::MissingField)?;
+        let path = header.path().ok_or(crate::Error::MissingField)?;
         let root = server.root().read().await;
         let node = root
             .get_child(path)
@@ -126,7 +126,7 @@ impl Properties {
         #[zbus(object_server)] server: &ObjectServer,
         #[zbus(header)] header: Header<'_>,
     ) -> Result<OwnedValue> {
-        let path = header.path()?.ok_or(crate::Error::MissingField)?;
+        let path = header.path().ok_or(crate::Error::MissingField)?;
         let root = server.root().read().await;
         let iface = root
             .get_child(path)
@@ -152,7 +152,7 @@ impl Properties {
         #[zbus(header)] header: Header<'_>,
         #[zbus(signal_context)] ctxt: SignalContext<'_>,
     ) -> Result<()> {
-        let path = header.path()?.ok_or(crate::Error::MissingField)?;
+        let path = header.path().ok_or(crate::Error::MissingField)?;
         let root = server.root().read().await;
         let iface = root
             .get_child(path)
@@ -190,7 +190,7 @@ impl Properties {
         #[zbus(object_server)] server: &ObjectServer,
         #[zbus(header)] header: Header<'_>,
     ) -> Result<HashMap<String, OwnedValue>> {
-        let path = header.path()?.ok_or(crate::Error::MissingField)?;
+        let path = header.path().ok_or(crate::Error::MissingField)?;
         let root = server.root().read().await;
         let iface = root
             .get_child(path)
@@ -292,7 +292,7 @@ impl ObjectManager {
         #[zbus(object_server)] server: &ObjectServer,
         #[zbus(header)] header: Header<'_>,
     ) -> Result<ManagedObjects> {
-        let path = header.path()?.ok_or(crate::Error::MissingField)?;
+        let path = header.path().ok_or(crate::Error::MissingField)?;
         let root = server.root().read().await;
         let node = root
             .get_child(path)

zbus/src/lib.rs

@@ -70,16 +70,6 @@ pub use message::Builder as MessageBuilder;
 #[deprecated(note = "Use `message::EndianSig` instead")]
 #[doc(hidden)]
 pub use message::EndianSig;
-#[deprecated(note = "Use `message::Field` instead")]
-#[doc(hidden)]
-pub use message::Field as MessageField;
-#[deprecated(note = "Use `message::FieldCode` instead")]
-#[doc(hidden)]
-pub use message::FieldCode as MessageFieldCode;
-#[deprecated(note = "Use `message::Fields` instead")]
-#[doc(hidden)]
-pub use message::Fields as MessageFields;
-#[deprecated(note = "Use `message::Flags` instead")]
 #[doc(hidden)]
 pub use message::Flags as MessageFlags;
 #[deprecated(note = "Use `message::Header` instead")]
@@ -253,9 +243,10 @@ mod tests {
             &(),
         )
         .unwrap();
-        assert_eq!(m.path().unwrap(), "/org/freedesktop/DBus");
-        assert_eq!(m.interface().unwrap(), "org.freedesktop.DBus.Peer");
-        assert_eq!(m.member().unwrap(), "GetMachineId");
+        let hdr = m.header();
+        assert_eq!(hdr.path().unwrap(), "/org/freedesktop/DBus");
+        assert_eq!(hdr.interface().unwrap(), "org.freedesktop.DBus.Peer");
+        assert_eq!(hdr.member().unwrap(), "GetMachineId");
         m.modify_primary_header(|primary| {
             primary.set_flags(BitFlags::from(Flags::NoAutoStart));
             primary.set_serial_num(11.try_into().unwrap());
@@ -720,9 +711,9 @@ mod tests {
 
             for m in stream {
                 let msg = m.unwrap();
-                let hdr = msg.header().unwrap();
+                let hdr = msg.header();
 
-                if hdr.member().unwrap().map(|m| m.as_str()) == Some("ZBusIssue122") {
+                if hdr.member().map(|m| m.as_str()) == Some("ZBusIssue122") {
                     break;
                 }
             }
@@ -950,9 +941,9 @@ mod tests {
 
         let mut stream = zbus::MessageStream::from(connection);
         while let Some(msg) = stream.try_next().await? {
-            let msg_header = msg.header()?;
+            let msg_header = msg.header();
 
-            match msg_header.message_type()? {
+            match msg_header.message_type() {
                 zbus::message::Type::MethodCall => {
                     connection.reply(&msg, &()).await?;
 

zbus/src/match_rule/mod.rs

@@ -1,6 +1,5 @@
 //! Bus match rule API.
 
-use core::panic;
 use std::ops::Deref;
 
 use serde::{de, Deserialize, Serialize};
@@ -207,7 +206,7 @@ impl<'m> MatchRule<'m> {
     /// * `destination` in the rule when `destination` on the `msg` is a well-known name. The
     ///   `destination` on match rule is always a unique name.
     pub fn matches(&self, msg: &zbus::message::Message) -> Result<bool> {
-        let hdr = msg.header()?;
+        let hdr = msg.header();
 
         // Start with message type.
         if let Some(msg_type) = self.msg_type() {
@@ -219,7 +218,7 @@ impl<'m> MatchRule<'m> {
         // Then check sender.
         if let Some(sender) = self.sender() {
             match sender {
-                BusName::Unique(name) if Some(name) != hdr.sender()? => {
+                BusName::Unique(name) if Some(name) != hdr.sender() => {
                     return Ok(false);
                 }
                 BusName::Unique(_) => (),
@@ -230,7 +229,7 @@ impl<'m> MatchRule<'m> {
 
         // The interface.
         if let Some(interface) = self.interface() {
-            match msg.interface().as_ref() {
+            match hdr.interface() {
                 Some(msg_interface) if interface != msg_interface => return Ok(false),
                 Some(_) => (),
                 None => return Ok(false),
@@ -239,7 +238,7 @@ impl<'m> MatchRule<'m> {
 
         // The member.
         if let Some(member) = self.member() {
-            match msg.member().as_ref() {
+            match hdr.member() {
                 Some(msg_member) if member != msg_member => return Ok(false),
                 Some(_) => (),
                 None => return Ok(false),
@@ -248,7 +247,7 @@ impl<'m> MatchRule<'m> {
 
         // The destination.
         if let Some(destination) = self.destination() {
-            match hdr.destination()? {
+            match hdr.destination() {
                 Some(BusName::Unique(name)) if destination != name => {
                     return Ok(false);
                 }
@@ -260,12 +259,12 @@ impl<'m> MatchRule<'m> {
 
         // The path.
         if let Some(path_spec) = self.path_spec() {
-            let msg_path = match msg.path() {
+            let msg_path = match hdr.path() {
                 Some(p) => p,
                 None => return Ok(false),
             };
             match path_spec {
-                PathSpec::Path(path) if path != &msg_path => return Ok(false),
+                PathSpec::Path(path) if path != msg_path => return Ok(false),
                 PathSpec::PathNamespace(path_ns) if !msg_path.starts_with(path_ns.as_str()) => {
                     return Ok(false);
                 }
@@ -330,7 +329,6 @@ impl ToString for MatchRule<'_> {
         if let Some(msg_type) = self.msg_type() {
             let type_str = match msg_type {
                 Type::Error => "error",
-                Type::Invalid => panic!("invalid message type"),
                 Type::MethodCall => "method_call",
                 Type::MethodReturn => "method_return",
                 Type::Signal => "signal",

zbus/src/message/builder.rs

@@ -93,7 +93,7 @@ impl<'a> Builder<'a> {
     ///
     /// The function will return an error if invalid flags are given for the message type.
     pub fn with_flags(mut self, flag: Flags) -> Result<Self> {
-        if self.header.message_type()? != Type::MethodCall
+        if self.header.message_type() != Type::MethodCall
             && BitFlags::from_flag(flag).contains(Flags::NoReplyExpected)
         {
             return Err(Error::InvalidField);
@@ -178,7 +178,7 @@ impl<'a> Builder<'a> {
         let serial = reply_to.primary().serial_num().ok_or(Error::MissingField)?;
         self.header.fields_mut().replace(Field::ReplySerial(serial));
 
-        if let Some(sender) = reply_to.sender()? {
+        if let Some(sender) = reply_to.sender() {
             self.destination(sender.to_owned())
         } else {
             Ok(self)

zbus/src/message/field.rs

@@ -21,9 +21,7 @@ use zvariant::{ObjectPath, Signature, Type, Value};
 /// [`Fields`]: struct.Fields.html
 #[repr(u8)]
 #[derive(Copy, Clone, Debug, Deserialize_repr, PartialEq, Eq, Serialize_repr, Type)]
-pub enum FieldCode {
-    /// Code for [`Field::Invalid`](enum.Field.html#variant.Invalid)
-    Invalid = 0,
+pub(super) enum FieldCode {
     /// Code for [`Field::Path`](enum.Field.html#variant.Path)
     Path = 1,
     /// Code for [`Field::Interface`](enum.Field.html#variant.Interface)
@@ -46,23 +44,6 @@ pub enum FieldCode {
 
 assert_impl_all!(FieldCode: Send, Sync, Unpin);
 
-impl From<u8> for FieldCode {
-    fn from(val: u8) -> FieldCode {
-        match val {
-            1 => FieldCode::Path,
-            2 => FieldCode::Interface,
-            3 => FieldCode::Member,
-            4 => FieldCode::ErrorName,
-            5 => FieldCode::ReplySerial,
-            6 => FieldCode::Destination,
-            7 => FieldCode::Sender,
-            8 => FieldCode::Signature,
-            9 => FieldCode::UnixFDs,
-            _ => FieldCode::Invalid,
-        }
-    }
-}
-
 impl<'f> Field<'f> {
     /// Get the associated code for this field.
     pub fn code(&self) -> FieldCode {
@@ -76,7 +57,6 @@ impl<'f> Field<'f> {
             Field::Sender(_) => FieldCode::Sender,
             Field::Signature(_) => FieldCode::Signature,
             Field::UnixFDs(_) => FieldCode::UnixFDs,
-            Field::Invalid => FieldCode::Invalid,
         }
     }
 }
@@ -93,9 +73,7 @@ impl<'f> Field<'f> {
 /// [are fixed]: struct.PrimaryHeader.html
 /// [Message Format]: https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-messages
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub enum Field<'f> {
-    /// Not a valid field.
-    Invalid,
+pub(super) enum Field<'f> {
     /// The object to send a call to, or the object a signal is emitted from.
     Path(ObjectPath<'f>),
     /// The interface to invoke a method call on, or that a signal is emitted from.
@@ -139,8 +117,6 @@ impl<'f> Serialize for Field<'f> {
             Field::Sender(value) => (FieldCode::Sender, value.as_str().into()),
             Field::Signature(value) => (FieldCode::Signature, value.as_ref().into()),
             Field::UnixFDs(value) => (FieldCode::UnixFDs, (*value).into()),
-            // This is a programmer error
-            Field::Invalid => panic!("Attempt to serialize invalid Field"),
         };
 
         tuple.serialize(serializer)
@@ -186,12 +162,6 @@ impl<'de: 'f, 'f> Deserialize<'de> for Field<'f> {
                 Field::Signature(Signature::try_from(value).map_err(D::Error::custom)?)
             }
             FieldCode::UnixFDs => Field::UnixFDs(u32::try_from(value).map_err(D::Error::custom)?),
-            FieldCode::Invalid => {
-                return Err(Error::invalid_value(
-                    serde::de::Unexpected::Unsigned(code as u64),
-                    &"A valid D-Bus message field code",
-                ));
-            }
         })
     }
 }