Wider and more precise error handling

Handle `InvalidArgs` separately from `Failed`

Change-Id: I01049d2bbea575121e30950f4ff63137cad26e96

diff --git a/sessiond/src/main.cpp b/sessiond/src/main.cpp
index 9e7fac7c8d686ba33bc062e0268c5763cf0a93bf..c933077c13596d8e2eccb873b6eed5f21d48e26a 100644 (file)
--- a/sessiond/src/main.cpp
+++ b/sessiond/src/main.cpp
@@ -137,7 +137,7 @@ struct wait_manager {
        void on_client_register(const std::string &name)
        {
                if (registered.contains(name))
-                       return; // TODO: any error here?
+                       throw std::invalid_argument("Client already registered for waiting");
                auto match = g_bus_watch_name(G_BUS_TYPE_SYSTEM, name.data(), G_BUS_NAME_WATCHER_FLAGS_NONE,
                                nullptr, glib_client_disappeared, this, nullptr);
                registered.emplace(name, std::move(match));
@@ -155,6 +155,9 @@ struct wait_manager {
 
                if (waiting.contains(v))
                        return; // TODO: any error here?
+                               // This shouldn't happen, but it's unclear how to punish this correctly.
+                               // In particular, this will not happen by construction in case of switch_user,
+                               // since v contains the switch_id.
 
                std::unordered_set<std::string> waits = {};
                for (const auto &going_to_wait : registered)
@@ -176,7 +179,7 @@ struct wait_manager {
        void on_client_done(std::tuple<Vs...> v, const std::string &name)
        {
                if (!waiting.contains(v))
-                       return; // TODO: any error here?
+                       return; // This could be an error, but then we would have a race condition with the timeout.
                drop_name(v, waiting[v], name);
        }
 
@@ -479,24 +482,22 @@ struct sessiond_context {
        static void glib_method_call(GDBusConnection *conn, const gchar *sender, const gchar *object_path,
                                     const gchar *interface_name, const gchar *method_name, GVariant *parameters,
                                     GDBusMethodInvocation *invocation, gpointer user_data)
-       {
-               // TODO: Also dispatch on interface_name (not needed at the moment)
-               try {
-                       auto self = static_cast<sessiond_context *>(user_data);
-                       auto to_call = std::find_if(methods.begin(), methods.end(), [&](auto pair) {
-                               return pair.first == method_name;
-                       });
-                       if (to_call == methods.end())
-                               throw std::runtime_error(std::string("Unknown method ") + method_name + " called");
-                       std::cout << "Handling " << method_name << " call from " << sender << std::endl;
-                       (self->*(to_call->second))(invocation, std::string_view(sender), parameters);
-               }
-               catch (const std::runtime_error &ex) {
-                       g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.Failed",
-                               (std::string("Unable to complete requested operation: ") + std::string(ex.what())).c_str());
-                       log_exception(ex, sender, method_name);
-                       // Swallow the exception; the show must go on
-               }
+       try { // TODO: Also dispatch on interface_name (not needed at the moment)
+               auto self = static_cast<sessiond_context *>(user_data);
+               auto to_call = std::find_if(methods.begin(), methods.end(), [&](auto pair) {
+                       return pair.first == method_name;
+               });
+               if (to_call == methods.end())
+                       throw std::runtime_error(std::string("Unknown method ") + method_name + " called");
+               std::cout << "Handling " << method_name << " call from " << sender << std::endl;
+               (self->*(to_call->second))(invocation, std::string_view(sender), parameters);
+       } catch (const std::invalid_argument &ex) {
+               g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", ex.what());
+       } catch (const std::runtime_error &ex) {
+               g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.Failed",
+                       (std::string("Unable to complete requested operation: ") + std::string(ex.what())).c_str());
+               log_exception(ex, sender, method_name);
+               // Swallow the exception; the show must go on
        }
        static void log_exception(const std::exception &ex, std::string_view sender, std::string_view method_name)
        {