Throw custom exceptions on D-Bus method failures

All 3 fan applications - control, monitor, and presence
have cases where it is expected that a getProperty call
may fail because a sensor is missing.  While the applications
already handle this, the InternalFailure exception that was
being thrown by the underlying call generates log entries
that make it look like something bad happened.

The custom exceptions now being thrown do not log anything on
creation, but store all of the failing information so that
any callers could still log the info if they wanted to.

Tested: Boot a water cooled Witherspoon and see the fan presence
        and monitor applications not look like they are
        failing.  Boot a system without the fan hwmon running,
        and see fan-control-init still show the fails.

Change-Id: Ifd8ad6e3deb492bbaf33f12c7258125dce1e5ea8
Signed-off-by: Matt Spinler <spinler@us.ibm.com>

6 files changed, 130 insertions, 44 deletions

diff --git a/control/functor.hpp b/control/functor.hpp
index 0a2b955..2ca047c 100644
--- a/control/functor.hpp
+++ b/control/functor.hpp
@@ -15,8 +15,6 @@ class Zone;
 using namespace phosphor::fan;
 using namespace sdbusplus::bus::match;
 using namespace phosphor::logging;
-using InternalFailure = sdbusplus::xyz::openbmc_project::Common::
-                                Error::InternalFailure;
 
 /**
  * @brief Create a handler function object
@@ -115,7 +113,7 @@ struct PropertyChanged
                                                            _property);
                 _handler(zone, std::forward<T>(val));
             }
-            catch (const InternalFailure& ife)
+            catch (const util::DBusError& e)
             {
                 // Property will not be used unless a property changed
                 // signal message is received for this property.
@@ -397,7 +395,7 @@ struct NameOwnerChanged
                         "NameHasOwner",
                         name);
             }
-            catch (const InternalFailure& ife)
+            catch (const util::DBusMethodError& e)
             {
                 // Failed to get service name owner state
                 hasOwner = false;
diff --git a/control/main.cpp b/control/main.cpp
index cde71d0..dd9330d 100644
--- a/control/main.cpp
+++ b/control/main.cpp
@@ -18,6 +18,7 @@
 #include "argument.hpp"
 #include "manager.hpp"
 #include "event.hpp"
+#include "sdbusplus.hpp"
 
 using namespace phosphor::fan::control;
 using namespace phosphor::logging;
@@ -64,22 +65,41 @@ int main(int argc, char* argv[])
     //handle both sd_events (for the timers) and dbus signals.
     bus.attach_event(eventPtr.get(), SD_EVENT_PRIORITY_NORMAL);
 
-    Manager manager(bus, eventPtr, mode);
+    try
+    {
+        Manager manager(bus, eventPtr, mode);
 
-    //Init mode will just set fans to max and delay
-    if (mode == Mode::init)
+        //Init mode will just set fans to max and delay
+        if (mode == Mode::init)
+        {
+            manager.doInit();
+            return 0;
+        }
+        else
+        {
+            r = sd_event_loop(eventPtr.get());
+            if (r < 0)
+            {
+                log<level::ERR>("Failed call to sd_event_loop",
+                        entry("ERROR=%s", strerror(-r)));
+            }
+        }
+    }
+    //Log the useful metadata on these exceptions and let the app
+    //return -1 so it is restarted without a core dump.
+    catch (phosphor::fan::util::DBusServiceError& e)
     {
-        manager.doInit();
-        return 0;
+        log<level::ERR>("Uncaught DBus service lookup failure exception",
+                entry("PATH=%s", e.path.c_str()),
+                entry("INTERFACE=%s", e.interface.c_str()));
     }
-    else
+    catch (phosphor::fan::util::DBusMethodError& e)
     {
-        r = sd_event_loop(eventPtr.get());
-        if (r < 0)
-        {
-            log<level::ERR>("Failed call to sd_event_loop",
-                            entry("ERROR=%s", strerror(-r)));
-        }
+        log<level::ERR>("Uncaught DBus method failure exception",
+                entry("BUSNAME=%s", e.busName.c_str()),
+                entry("PATH=%s", e.path.c_str()),
+                entry("INTERFACE=%s", e.interface.c_str()),
+                entry("METHOD=%s", e.method.c_str()));
     }
 
     return -1;
diff --git a/control/matches.hpp b/control/matches.hpp
index ccacbc4..c3b4006 100644
--- a/control/matches.hpp
+++ b/control/matches.hpp
@@ -14,8 +14,6 @@ namespace match
 
 using namespace phosphor::fan;
 using namespace sdbusplus::bus::match;
-using InternalFailure = sdbusplus::xyz::openbmc_project::Common::
-                             Error::InternalFailure;
 
 /**
  * @brief A match function that constructs a PropertiesChanged match string
@@ -77,7 +75,7 @@ inline auto nameOwnerChanged(const std::string& obj, const std::string& iface)
     {
         noc = rules::nameOwnerChanged(util::SDBusPlus::getService(obj, iface));
     }
-    catch (const InternalFailure& ife)
+    catch (const util::DBusError& e)
     {
         // Unable to construct NameOwnerChanged match string
     }
diff --git a/control/zone.cpp b/control/zone.cpp
index 325a5b8..f4c50d9 100644
--- a/control/zone.cpp
+++ b/control/zone.cpp
@@ -196,7 +196,7 @@ void Zone::setServices(const Group* group)
                     "NameHasOwner",
                     name);
         }
-        catch (const InternalFailure& ife)
+        catch (const util::DBusMethodError& e)
         {
             // Failed to get service name owner state
             hasOwner = false;
diff --git a/monitor/tach_sensor.cpp b/monitor/tach_sensor.cpp
index 5e985d0..6789ed8 100644
--- a/monitor/tach_sensor.cpp
+++ b/monitor/tach_sensor.cpp
@@ -108,6 +108,8 @@ TachSensor::TachSensor(Mode mode,
         }
         catch (std::exception& e)
         {
+            log<level::INFO>("Not monitoring a tach sensor",
+                    entry("SENSOR=%s", _name.c_str()));
             throw InvalidSensorError();
         }
 
diff --git a/sdbusplus.hpp b/sdbusplus.hpp
index 454fde8..e977744 100644
--- a/sdbusplus.hpp
+++ b/sdbusplus.hpp
@@ -19,6 +19,74 @@ namespace detail
 namespace errors = sdbusplus::xyz::openbmc_project::Common::Error;
 } // namespace detail
 
+/**
+ * @class DBusError
+ *
+ * The base class for the exceptions thrown on fails in the various
+ * SDBusPlus calls.  Used so that a single catch statement can catch
+ * any type of these exceptions.
+ *
+ * None of these exceptions will log anything when they are created,
+ * it is up to the handler to do that if desired.
+ */
+class DBusError : public std::runtime_error
+{
+    public:
+        DBusError(const char* msg) :
+            std::runtime_error(msg)
+        {
+        }
+};
+
+/**
+ * @class DBusMethodError
+ *
+ * Thrown on a DBus Method call failure
+ */
+class DBusMethodError : public DBusError
+{
+    public:
+        DBusMethodError(
+                const std::string& busName,
+                const std::string& path,
+                const std::string& interface,
+                const std::string& method) :
+            DBusError("DBus method call failed"),
+            busName(busName),
+            path(path),
+            interface(interface),
+            method(method)
+            {
+            }
+
+        const std::string busName;
+        const std::string path;
+        const std::string interface;
+        const std::string method;
+};
+
+/**
+ * @class DBusServiceError
+ *
+ * Thrown when a service lookup fails.  Usually this points to
+ * the object path not being present in D-Bus.
+ */
+class DBusServiceError : public DBusError
+{
+    public:
+        DBusServiceError(
+                const std::string& path,
+                const std::string& interface) :
+            DBusError("DBus service lookup failed"),
+            path(path),
+            interface(interface)
+            {
+            }
+
+        const std::string path;
+        const std::string interface;
+};
+
 /** @brief Alias for PropertiesChanged signal callbacks. */
 template <typename ...T>
 using Properties = std::map<std::string, sdbusplus::message::variant<T...>>;
@@ -57,13 +125,7 @@ class SDBusPlus
 
             if (respMsg.is_method_error())
             {
-                phosphor::logging::log<phosphor::logging::level::INFO>(
-                        "Failed to invoke DBus method.",
-                        phosphor::logging::entry("PATH=%s", path.c_str()),
-                        phosphor::logging::entry(
-                                "INTERFACE=%s", interface.c_str()),
-                        phosphor::logging::entry("METHOD=%s", method.c_str()));
-                phosphor::logging::elog<detail::errors::InternalFailure>();
+                throw DBusMethodError{busName, path, interface, method};
             }
 
             return respMsg;
@@ -176,25 +238,31 @@ class SDBusPlus
             using namespace std::literals::string_literals;
             using GetObject = std::map<std::string, std::vector<std::string>>;
 
-            auto mapperResp = callMethodAndRead<GetObject>(
-                    bus,
-                    "xyz.openbmc_project.ObjectMapper"s,
-                    "/xyz/openbmc_project/object_mapper"s,
-                    "xyz.openbmc_project.ObjectMapper"s,
-                    "GetObject"s,
-                    path,
-                    GetObject::mapped_type{interface});
-
-            if (mapperResp.empty())
+            try
             {
-                phosphor::logging::log<phosphor::logging::level::INFO>(
-                        "Object not found.",
-                        phosphor::logging::entry("PATH=%s", path.c_str()),
-                        phosphor::logging::entry(
-                                "INTERFACE=%s", interface.c_str()));
-                phosphor::logging::elog<detail::errors::InternalFailure>();
+                auto mapperResp = callMethodAndRead<GetObject>(
+                        bus,
+                        "xyz.openbmc_project.ObjectMapper"s,
+                        "/xyz/openbmc_project/object_mapper"s,
+                        "xyz.openbmc_project.ObjectMapper"s,
+                        "GetObject"s,
+                        path,
+                        GetObject::mapped_type{interface});
+
+                if (mapperResp.empty())
+                {
+                    //Should never happen.  A missing object would fail
+                    //in callMethodAndRead()
+                    phosphor::logging::log<phosphor::logging::level::ERR>(
+                            "Empty mapper response on service lookup");
+                    throw DBusServiceError{path, interface};
+                }
+                return mapperResp.begin()->first;
+            }
+            catch (DBusMethodError& e)
+            {
+                throw DBusServiceError{path, interface};
             }
-            return mapperResp.begin()->first;
         }
 
         /** @brief Get service from the mapper. */