sdbusplus exception type SdBusError
Patrick Williams
patrick at stwcx.xyz
Fri Sep 3 01:36:28 AEST 2021
Hello,
I am going to start on some changes to sdbusplus which make it so that some
errors which previously threw an 'SdBusError' now throw a more specialized type.
In the process of getting started on this I'm observing a few existing issues
across the codebase.
1. Many repositories were catching 'SdBusError'. This type was always
intended to be internal to sdbusplus (in fact, it inherits from an error
called 'internal_error'). Quite likely, by catching this exception, you
are missing other exceptions that sdbusplus can throw. Certainly you are
missing exceptions that sdbusplus will start throwing with my upcoming
changes.
2. I see a handful of repositories *throwing* SdBusErrors. Again, this was
intended to be an internal exception to sdbusplus itself and not a generic
exception type that any arbitrary string could be wrapped with. I'm going
to leave this alone for the time being, but in the future it is quite
likely that I'll force a compile break with this. If you need to throw
something that is of type sdbusplus::exception::exception and can't figure
out what is better, please reach out to me with some context.
3. Along the lines of #2, *some* of the cases where an SdBusError is thrown
is entirely invalid error path handling and your application is _GOING_
_TO_ _CRASH_ whenever you have a dbus client that pushes you down these
paths! Almost all of these I see are using the ASIO bindings. In many
cases the ASIO bindings *DO NOT* catch any exceptions and magically turn
them into sd_bus_error's. By throwing an exception, you're not making an
error return to the calling client, but instead blowing through all of the
sd_bus C code with your C++ exception and putting your application into an
invalid state. At a minimum you are leaking memory.
*(3): the phosphor-dbus-interface bindings *DO* turn exceptions into
sd_bus_error responses but typically this requires you to throw a PDI
error type.
I have put up a large number of commits to fix #1, so if all you are doing is
catching SdBusError you probably have no change necessary. If you see a
`catch (SdBusError&)` being added to the codebase, know it is probably wrong and
should be fixed in review.
--
Patrick Williams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20210902/7be75fe0/attachment.sig>
More information about the openbmc
mailing list