Skip to content

Commit

Permalink
🥅 zb: don't assert on invalid serve_at() usage
Browse files Browse the repository at this point in the history
Document that standard interfaces are already added.

Return an error instead of an explicit panic if attempted, since it is
not an internal invariant.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
  • Loading branch information
elmarco committed Apr 28, 2024
1 parent 91abff7 commit aafc9e1
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
17 changes: 10 additions & 7 deletions zbus/src/connection/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ impl<'a> Builder<'a> {
/// interfaces available immediately after the connection is established. Typically, this is
/// exactly what you'd want. Also in contrast to [`zbus::ObjectServer::at`], this method will
/// replace any previously added interface with the same name at the same path.
///
/// Standard interfaces (Peer, Introspectable, Properties) are added on your behalf. If you
/// attempt to add yours, [`Builder::build()`] will fail.
pub fn serve_at<P, I>(mut self, path: P, iface: I) -> Result<Self>
where
I: Interface,
Expand Down Expand Up @@ -448,13 +451,13 @@ impl<'a> Builder<'a> {
for (path, interfaces) in self.interfaces {
for (name, iface) in interfaces {
let iface = iface.clone();
let future =
object_server
.inner()
.at_ready(path.to_owned(), name.clone(), || iface);
let added = future.await?;
// Duplicates shouldn't happen.
assert!(added);
let added = object_server
.inner()
.at_ready(path.to_owned(), name.clone(), || iface)
.await?;
if !added {
return Err(Error::InterfaceExists(name.clone(), path.to_owned()));
}
}
}

Expand Down
10 changes: 8 additions & 2 deletions zbus/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use static_assertions::assert_impl_all;
use std::{convert::Infallible, error, fmt, io, sync::Arc};
use zbus_names::{Error as NamesError, OwnedErrorName};
use zvariant::Error as VariantError;
use zbus_names::{Error as NamesError, InterfaceName, OwnedErrorName};
use zvariant::{Error as VariantError, ObjectPath};

use crate::{
fdo,
Expand Down Expand Up @@ -59,6 +59,8 @@ pub enum Error {
MissingParameter(&'static str),
/// Serial number in the message header is 0 (which is invalid).
InvalidSerial,
/// The given interface already exists at the given path.
InterfaceExists(InterfaceName<'static>, ObjectPath<'static>),
}

assert_impl_all!(Error: Send, Sync, Unpin);
Expand All @@ -85,6 +87,7 @@ impl PartialEq for Error {
(Self::NameTaken, Self::NameTaken) => true,
(Error::InputOutput(_), Self::InputOutput(_)) => false,
(Self::Failure(s1), Self::Failure(s2)) => s1 == s2,
(Self::InterfaceExists(s1, s2), Self::InterfaceExists(o1, o2)) => s1 == o1 && s2 == o2,
(_, _) => false,
}
}
Expand Down Expand Up @@ -113,6 +116,7 @@ impl error::Error for Error {
Error::Failure(_) => None,
Error::MissingParameter(_) => None,
Error::InvalidSerial => None,
Error::InterfaceExists(_, _) => None,
}
}
}
Expand Down Expand Up @@ -147,6 +151,7 @@ impl fmt::Display for Error {
write!(f, "Parameter `{}` was not specified but it is required", p)
}
Error::InvalidSerial => write!(f, "Serial number in the message header is 0"),
Error::InterfaceExists(i, p) => write!(f, "Interface `{i}` already exists at `{p}`"),
}
}
}
Expand Down Expand Up @@ -176,6 +181,7 @@ impl Clone for Error {
Error::Failure(e) => Error::Failure(e.clone()),
Error::MissingParameter(p) => Error::MissingParameter(p),
Error::InvalidSerial => Error::InvalidSerial,
Error::InterfaceExists(i, p) => Error::InterfaceExists(i.clone(), p.clone()),
}
}
}
Expand Down

0 comments on commit aafc9e1

Please sign in to comment.