diff --git a/docs/using-sdbus-c++.md b/docs/using-sdbus-c++.md index 0028953..feb7f08 100644 --- a/docs/using-sdbus-c++.md +++ b/docs/using-sdbus-c++.md @@ -592,6 +592,7 @@ We recommend that sdbus-c++ users prefer the convenience API to the lower level, > assert(e->getMessage() == "Failed to deserialize a int32 value"); > } > ``` +> Signature mismatch in signal handlers is probably the most common reason why signals are not received in the client, while we can see them on the bus with `dbus-monitor`. Use `const sdbus::Error*`-based callback variant and inspect the error to check if that's the cause of such problems. > **_Tip_:** When registering a D-Bus object, we can additionally provide names of input and output parameters of its methods and names of parameters of its signals. When the object is introspected, these names are listed in the resulting introspection XML, which improves the description of object's interfaces: > ```c++ diff --git a/include/sdbus-c++/ConvenienceApiClasses.inl b/include/sdbus-c++/ConvenienceApiClasses.inl index 1ffc37a..f84dc93 100644 --- a/include/sdbus-c++/ConvenienceApiClasses.inl +++ b/include/sdbus-c++/ConvenienceApiClasses.inl @@ -595,9 +595,8 @@ namespace sdbus { } catch (const Error& e) { - // Catch message unpack exceptions and pass them to the callback - // in the expected manner to avoid propagating them up the call - // stack to the event loop. + // Pass message deserialization exceptions to the client via callback error parameter, + // instead of propagating them up the message loop call stack. sdbus::apply(callback, &e, args); return; } @@ -676,8 +675,10 @@ namespace sdbus { } catch (const sdbus::Error& e) { - // Invoke callback with error argument and input arguments from the tuple. + // Pass message deserialization exceptions to the client via callback error parameter, + // instead of propagating them up the message loop call stack. sdbus::apply(callback, &e, signalArgs); + return; } // Invoke callback with no error and input arguments from the tuple. diff --git a/include/sdbus-c++/Error.h b/include/sdbus-c++/Error.h index 709cc17..51d5d3e 100644 --- a/include/sdbus-c++/Error.h +++ b/include/sdbus-c++/Error.h @@ -76,6 +76,8 @@ namespace sdbus { }; sdbus::Error createError(int errNo, const std::string& customMsg); + + inline const char* SDBUSCPP_ERROR_NAME = "org.sdbuscpp.Error"; } #define SDBUS_THROW_ERROR(_MSG, _ERRNO) \ diff --git a/src/Object.cpp b/src/Object.cpp index c6df821..af1ecf1 100644 --- a/src/Object.cpp +++ b/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(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; } } diff --git a/src/Proxy.cpp b/src/Proxy.cpp index 983cffe..933699b 100644 --- a/src/Proxy.cpp +++ b/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(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(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; } } diff --git a/src/Utils.h b/src/Utils.h index 687b37c..e2913e2 100644 --- a/src/Utils.h +++ b/src/Utils.h @@ -50,4 +50,34 @@ #define SDBUS_CHECK_MEMBER_NAME(_NAME) #endif +namespace sdbus::internal { + + template + 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_ */