Reg sdbusplus - async handlers - D-Bus error not getting reflected

[Thomaiyar, Richard Marian]

Hi Lei,

https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/21611  is exactly what i had in mind for #2 but still looking for #1, so that error_no can be propagated, at this point of time, it is not set from D-Bus daemon.

https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/22481 - is merged, and it solves async_send, but doesn't work for async_method_call(). Any situation in which you have used in async_method_call() ?

regards,
Richard

On 6/26/2019 12:40 PM, Lei YU wrote:
> On Wed, Jun 26, 2019 at 12:19 AM Thomaiyar, Richard Marian
> <richard.marian.thomaiyar at linux.intel.com> wrote:
>> Hi,
>>
>> In sdbusplus code, async_send_handler callback() uses the
>> sdbusplus::message::message.get_errorno()
>> https://github.com/openbmc/sdbusplus/blob/master/sdbusplus/asio/detail/async_send_handler.hpp#L66
>> function to instantiate the boost::system::error_code. Unfortunately,
>> none of our D-Bus exception throwing functionality sets the error_no to
>> a proper one, but instead updates only the error_message field
>> https://github.com/openbmc/sdbusplus/blob/master/tools/sdbusplus/templates/method.mako.prototype.hpp.in#L171
>> causing the error_code instance to always return a generic error code
>> value, instead of a proper one. Because of this applications which uses
>> the asio logic will not be able to differentiate between exceptions
>> thrown / errors from D-Bus.
>>
>> Planning to fix the same in 2 step,
>>
>> 1. To make all D-Bus exception to set the errorno properly using
>> sd_bus_error_set_errno and getting the error from the yaml if available
>> else return generic one (so that no change in existing daemon will be
>> required).
>>
>> 2. make change in async_send_handler_callaback, such that derived class
>> of boost::system::error_code is returned, which will hold the error
>> message too. Any daemon which uses asio logic, can depend on ec.value()
>> as primary exception identifier or ec.message() for any detailed
>> exception thrown.
>>
>> Let me know your thoughts , and if agree, will start implementing the same.
>>
>> Regards,
>>
>> Richard
> I am glad that you feel the same way!
> The patch https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/21611 was
> submitted trying to resolve the issue.
> And during review, it's noticed that there is a better way to achieve the goal,
> that the DBus error is returned by the message.
>
> So we have https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/22481
>
> Now in sdbusplus, message::get_error() is added, and the caller could get the
> sd_bus_error* from the returned message, and get the DBus exception details
> from the sd_bus_error.
>
> See examples at
> https://github.com/openbmc/bmcweb/blob/e4a4b9a95622b8e1c1bae93718699ad19f4882ac/include/openbmc_dbus_rest.hpp#L1385-L1403
> https://github.com/openbmc/bmcweb/blob/e4a4b9a95622b8e1c1bae93718699ad19f4882ac/include/openbmc_dbus_rest.hpp#L1882-L1893