From e3c082e3619ab2d9095b58e55c90157f6376fd1e Mon Sep 17 00:00:00 2001
From: Vendula Poncova <vponcova@redhat.com>
Date: Thu, 16 Jul 2020 20:24:25 +0200
Subject: [PATCH 2/2] Handle all errors of the DBus call

Don't handle only errors caused by the method invocation, but handle
also errors caused by processing the result of the method.
---
 dasbus/client/handler.py |  3 +--
 dasbus/server/handler.py | 11 +++++------
 tests/test_client.py     | 16 ++++++++++++++++
 tests/test_server.py     | 36 ++++++++++++++++++++++++++++++------
 4 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/dasbus/client/handler.py b/dasbus/client/handler.py
index ed1a690..839f5a6 100644
--- a/dasbus/client/handler.py
+++ b/dasbus/client/handler.py
@@ -472,10 +472,9 @@ class ClientObjectHandler(AbstractClientObjectHandler):
         """
         try:
             result = call(*args, **kwargs)
+            return self._handle_method_result(result)
         except Exception as error:  # pylint: disable=broad-except
             return self._handle_method_error(error)
-        else:
-            return self._handle_method_result(result)
 
     def _handle_method_error(self, error):
         """Handle an error of a DBus call.
diff --git a/dasbus/server/handler.py b/dasbus/server/handler.py
index 3871948..8183f26 100644
--- a/dasbus/server/handler.py
+++ b/dasbus/server/handler.py
@@ -420,6 +420,11 @@ class ServerObjectHandler(AbstractServerObjectHandler):
                 method_name,
                 *unwrap_variant(parameters)
             )
+            self._handle_method_result(
+                invocation,
+                member,
+                result
+            )
         except Exception as error:  # pylint: disable=broad-except
             self._handle_method_error(
                 invocation,
@@ -427,12 +432,6 @@ class ServerObjectHandler(AbstractServerObjectHandler):
                 method_name,
                 error
             )
-        else:
-            self._handle_method_result(
-                invocation,
-                member,
-                result
-            )
 
     def _handle_method_error(self, invocation, interface_name, method_name,
                              error):
diff --git a/tests/test_client.py b/tests/test_client.py
index 7cb96c3..cd44552 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -256,6 +256,22 @@ class DBusClientTestCase(unittest.TestCase):
             str(cm.exception)
         )
 
+    def test_invalid_method_result(self):
+        """Test a method proxy with an invalid result."""
+        self._create_proxy("""
+        <node>
+            <interface name="Interface">
+                <method name="Method">
+                    <arg direction="out" name="return" type="t"/>
+                </method>
+            </interface>
+        </node>
+        """)
+
+        self._set_reply(get_variant("i", -1))
+        with self.assertRaises(TypeError):
+            self.proxy.Method()
+
     def _set_reply(self, reply_value):
         """Set the reply of the DBus call."""
         self.connection.call_sync.reset_mock()
diff --git a/tests/test_server.py b/tests/test_server.py
index ce91b97..4adcf89 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -82,8 +82,8 @@ class DBusServerTestCase(unittest.TestCase):
 
     def _call_method_with_error(self, interface, method,
                                 parameters=NO_PARAMETERS,
-                                error_name="",
-                                error_message=""):
+                                error_name=None,
+                                error_message=None):
         invocation = Mock()
 
         with self.assertLogs(level='WARN'):
@@ -94,12 +94,17 @@ class DBusServerTestCase(unittest.TestCase):
                 parameters
             )
 
-        invocation.return_dbus_error.assert_called_once_with(
-            error_name,
-            error_message
-        )
+        invocation.return_dbus_error.assert_called_once()
         invocation.return_value.assert_not_called()
 
+        (name, msg), kwargs = invocation.return_dbus_error.call_args
+
+        self.assertEqual(kwargs, {})
+        self.assertEqual(name, error_name, "Unexpected error name.")
+
+        if error_message is not None:
+            self.assertEqual(msg, error_message, "Unexpected error message.")
+
     def test_register(self):
         """Test the object registration."""
         with self.assertRaises(DBusSpecificationError) as cm:
@@ -193,6 +198,25 @@ class DBusServerTestCase(unittest.TestCase):
             error_message="The method has failed."
         )
 
+    def test_invalid_method_result(self):
+        """Test a method with an invalid result."""
+        self._publish_object("""
+        <node>
+            <interface name="Interface">
+                <method name="Method">
+                    <arg direction="out" name="return" type="t"/>
+                </method>
+            </interface>
+        </node>
+        """)
+
+        self.object.Method.return_value = -1
+        self._call_method_with_error(
+            "Interface",
+            "Method",
+            error_name="not.known.Error.OverflowError"
+        )
+
     def test_property(self):
         """Test the property publishing."""
         self._publish_object("""
-- 
2.26.2