From ab2cae9c4b63d983635c3d933b0f56473636ecb9 Mon Sep 17 00:00:00 2001
From: Zeeshan Ali Khan <zeenix@gmail.com>
Date: Wed, 12 Jun 2024 17:34:33 +0200
Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A5=85=20zm:=20interface-generated=20?=
 =?UTF-8?q?proxy=20should=20use=20the=20same=20error=20types?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If user specifies a Result type as return type of a method, the generated
proxy should use the same error types. We might need to add an attribute
to control in the future, as this as this may not always be what the user
wants.

We should do this because if we hardcode the error type to be
`zbus::Error`, the API will have to break if one decides to replace a
`proxy` use with this new ability of `interface` and they had their
`proxy` return a specific error type. This for example is the case for
`zbus::fdo` API, which we want to convert in the future.

Speaking of API break, this change itself is an API break but
practically speaking the chances of someone already making use of the
newly added feature of generating proxy from interface is pretty low (on
the Matrix channel people didn't even know it was added), let alone them
using a custom error in interface methods while not in proxy as well.
---
 zbus_macros/src/iface.rs | 48 ++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/zbus_macros/src/iface.rs b/zbus_macros/src/iface.rs
index 8dd29dbc3..51112ab4f 100644
--- a/zbus_macros/src/iface.rs
+++ b/zbus_macros/src/iface.rs
@@ -778,7 +778,7 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
         }
 
         if let Some(proxy) = &mut proxy {
-            proxy.add_method(info, &properties);
+            proxy.add_method(info, &properties)?;
         }
     }
 
@@ -1335,7 +1335,11 @@ impl Proxy {
         }
     }
 
-    fn add_method(&mut self, method_info: MethodInfo, properties: &BTreeMap<String, Property<'_>>) {
+    fn add_method(
+        &mut self,
+        method_info: MethodInfo,
+        properties: &BTreeMap<String, Property<'_>>,
+    ) -> syn::Result<()> {
         let inputs: Punctuated<PatType, Comma> = method_info
             .typed_inputs
             .iter()
@@ -1345,9 +1349,38 @@ impl Proxy {
             })
             .cloned()
             .collect();
-        let ret = get_return_type(&method_info.output)
-            .map(|r| quote!(#r))
-            .unwrap_or(quote!(()));
+        let zbus = &self.zbus;
+        let ret = match &method_info.output {
+            ReturnType::Type(_, ty) => {
+                let ty = ty.as_ref();
+
+                if let Type::Path(p) = ty {
+                    let is_result_output = p
+                        .path
+                        .segments
+                        .last()
+                        .ok_or_else(|| Error::new_spanned(ty, "unsupported return type"))?
+                        .ident
+                        == "Result";
+                    if is_result_output {
+                        let is_prop = matches!(method_info.method_type, MethodType::Property(_));
+
+                        if is_prop {
+                            // Proxy methods always return `zbus::Result<T>`
+                            let inner_ty = get_result_inner_type(p)?;
+                            quote! { #zbus::Result<#inner_ty> }
+                        } else {
+                            quote! { #ty }
+                        }
+                    } else {
+                        quote! { #zbus::Result<#ty> }
+                    }
+                } else {
+                    quote! { #zbus::Result<#ty> }
+                }
+            }
+            ReturnType::Default => quote! { #zbus::Result<()> },
+        };
         let ident = &method_info.ident;
         let member_name = method_info.member_name;
         let mut proxy_method_attrs = quote! { name = #member_name, };
@@ -1386,14 +1419,15 @@ impl Proxy {
             }
         }
         let cfg_attrs = method_info.cfg_attrs;
-        let zbus = &self.zbus;
         let doc_attrs = method_info.doc_attrs;
         self.methods.extend(quote! {
             #(#cfg_attrs)*
             #(#doc_attrs)*
             #[zbus(#proxy_method_attrs)]
-            fn #ident(&self, #inputs) -> #zbus::Result<#ret>;
+            fn #ident(&self, #inputs) -> #ret;
         });
+
+        Ok(())
     }
 
     fn gen(&self) -> TokenStream {

From 88b652c07a5ba337db9e03a2363c8af435254228 Mon Sep 17 00:00:00 2001
From: Zeeshan Ali Khan <zeenix@gmail.com>
Date: Sat, 29 Jun 2024 16:22:17 +0200
Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=85=20zb:=20Check=20errors=20from=20f?=
 =?UTF-8?q?allible=20methods=20in=20e2e=20test?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 zbus/tests/e2e.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/zbus/tests/e2e.rs b/zbus/tests/e2e.rs
index 96b231aa5..f1a39ee31 100644
--- a/zbus/tests/e2e.rs
+++ b/zbus/tests/e2e.rs
@@ -84,7 +84,7 @@ impl MyIface {
 }
 
 /// Custom D-Bus error type.
-#[derive(Debug, DBusError)]
+#[derive(Debug, DBusError, PartialEq)]
 #[zbus(prefix = "org.freedesktop.MyIface.Error")]
 enum MyIfaceError {
     SomethingWentWrong(String),
@@ -569,6 +569,13 @@ async fn my_iface_test(conn: Connection, event: Event) -> zbus::Result<u32> {
             bar: "TestString".into(),
         })
         .await?;
+
+    proxy.test_error().await.unwrap_err();
+    assert_eq!(
+        proxy.test_custom_error().await.unwrap_err(),
+        MyIfaceError::SomethingWentWrong("oops".to_string())
+    );
+
     check_hash_map(proxy.test_hashmap_return().await?);
     check_hash_map(proxy.hash_map().await?);
     proxy