refactor: improve handling of exceptions from callback handlers (#375)

* Catch and process all exceptions (not just sdbus::Error) from callback handlers
* Unify handling of exceptions from all types of callbacks -- always set sd_bus_error and return a negative result number in case of exception

Although libsystemd logs (with DEBUG severity) all errors from such callback handlers (except method callback handler), it seems to be out of our control. One of handy sdbus-c++ features could be the ability for clients to install a log callback, which sdbus-c++ would call in case of exceptions flying from callback handlers. In case something doesn't work for clients (especially novices), they can first look into these logs.

This may be handy in common situations like ignored signals on client side because of the inadvertent mismatch between real signal signature and signal handler signature. Like here: #373. (Although in this specific case of signals, there is a solution with an additional const sdbus::Error* argument that would reveal such an error.)

src/Object.cpp

@@ -347,16 +347,9 @@ int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData,
     auto& callback = interfaceData->methods[message.getMemberName()].callback;
     assert(callback);
 
-    try
-    {
-        callback(message);
-    }
-    catch (const Error& e)
-    {
-        sd_bus_error_set(retError, e.getName().c_str(), e.getMessage().c_str());
-    }
+    auto ok = invokeHandlerAndCatchErrors([&](){ callback(message); }, retError);
 
-    return 1;
+    return ok ? 1 : -1;
 }
 
 int Object::sdbus_property_get_callback( sd_bus */*bus*/
@@ -381,16 +374,9 @@ int Object::sdbus_property_get_callback( sd_bus */*bus*/
 
     auto reply = Message::Factory::create<PropertyGetReply>(sdbusReply, &object.connection_.getSdBusInterface());
 
-    try
-    {
-        callback(reply);
-    }
-    catch (const Error& e)
-    {
-        sd_bus_error_set(retError, e.getName().c_str(), e.getMessage().c_str());
-    }
+    auto ok = invokeHandlerAndCatchErrors([&](){ callback(reply); }, retError);
 
-    return 1;
+    return ok ? 1 : -1;
 }
 
 int Object::sdbus_property_set_callback( sd_bus */*bus*/
@@ -416,16 +402,9 @@ int Object::sdbus_property_set_callback( sd_bus */*bus*/
         object.m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed);
     };
 
-    try
-    {
-        callback(value);
-    }
-    catch (const Error& e)
-    {
-        sd_bus_error_set(retError, e.getName().c_str(), e.getMessage().c_str());
-    }
+    auto ok = invokeHandlerAndCatchErrors([&](){ callback(value); }, retError);
 
-    return 1;
+    return ok ? 1 : -1;
 }
 
 }

src/Proxy.cpp

@@ -269,7 +269,7 @@ const Message* Proxy::getCurrentlyProcessedMessage() const
     return m_CurrentlyProcessedMessage.load(std::memory_order_relaxed);
 }
 
-int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error */*retError*/)
+int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError)
 {
     auto* asyncCallData = static_cast<AsyncCalls::CallData*>(userData);
     assert(asyncCallData != nullptr);
@@ -295,7 +295,7 @@ int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userDat
         proxy.m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed);
     };
 
-    try
+    auto ok = invokeHandlerAndCatchErrors([&]
     {
         const auto* error = sd_bus_message_get_error(sdbusMessage);
         if (error == nullptr)
@@ -307,16 +307,12 @@ int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userDat
             Error exception(error->name, error->message);
             asyncCallData->callback(message, &exception);
         }
-    }
-    catch (const Error&)
-    {
-        // Intentionally left blank -- sdbus-c++ exceptions shall not bubble up to the underlying C sd-bus library
-    }
+    }, retError);
 
-    return 0;
+    return ok ? 0 : -1;
 }
 
-int Proxy::sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error */*retError*/)
+int Proxy::sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError)
 {
     auto* signalData = static_cast<InterfaceData::SignalData*>(userData);
     assert(signalData != nullptr);
@@ -330,16 +326,9 @@ int Proxy::sdbus_signal_handler(sd_bus_message *sdbusMessage, void *userData, sd
         signalData->proxy.m_CurrentlyProcessedMessage.store(nullptr, std::memory_order_relaxed);
     };
 
-    try
-    {
-        signalData->callback(message);
-    }
-    catch (const Error&)
-    {
-        // Intentionally left blank -- sdbus-c++ exceptions shall not bubble up to the underlying C sd-bus library
-    }
+    auto ok = invokeHandlerAndCatchErrors([&](){ signalData->callback(message); }, retError);
 
-    return 0;
+    return ok ? 0 : -1;
 }
 
 }

src/Utils.h

@@ -50,4 +50,34 @@
 #define SDBUS_CHECK_MEMBER_NAME(_NAME)
 #endif
 
+namespace sdbus::internal {
+
+    template <typename _Callable>
+    bool invokeHandlerAndCatchErrors(_Callable callable, sd_bus_error *retError)
+    {
+        try
+        {
+            callable();
+        }
+        catch (const Error& e)
+        {
+            sd_bus_error_set(retError, e.getName().c_str(), e.getMessage().c_str());
+            return false;
+        }
+        catch (const std::exception& e)
+        {
+            sd_bus_error_set(retError, SDBUSCPP_ERROR_NAME, e.what());
+            return false;
+        }
+        catch (...)
+        {
+            sd_bus_error_set(retError, SDBUSCPP_ERROR_NAME, "Unknown error occurred");
+            return false;
+        }
+
+        return true;
+    }
+
+}
+
 #endif /* SDBUS_CXX_INTERNAL_UTILS_H_ */