Mercurial > irccd

changeset 570:153e84e7b09b

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Tests: add error code support in cmd-plugin-* tests
author David Demelier <markand@malikania.fr>
date Tue, 28 Nov 2017 17:17:30 +0100
parents 24b79bccc181
children 23fc81b1bd8f
files libirccd-js/irccd/js/js_plugin.cpp libirccd-test/irccd/command_test.hpp libirccd/irccd/command.cpp libirccd/irccd/command.hpp libirccd/irccd/plugin_service.cpp libirccd/irccd/plugin_service.hpp libirccd/irccd/rule.cpp libirccd/irccd/rule.hpp libirccd/irccd/rule_service.cpp tests/cmd-plugin-config/main.cpp tests/cmd-plugin-info/main.cpp tests/cmd-plugin-load/main.cpp tests/cmd-plugin-reload/main.cpp tests/cmd-plugin-unload/main.cpp
diffstat 14 files changed, 330 insertions(+), 47 deletions(-) [+]
line wrap: on
line diff
--- a/libirccd-js/irccd/js/js_plugin.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd-js/irccd/js/js_plugin.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -73,13 +73,19 @@
         // Function not defined, remove the undefined value and all arguments.
         duk_pop_n(context_, nargs + 1);
     else {
+        std::string error;
+
         // Call the function and discard the result.
         duk_insert(context_, -nargs - 1);
 
+        // TODO: convert into a human readable string.
         if (duk_pcall(context_, nargs) != 0)
-            throw dukx_stack(context_, -1, true);
+            error = duk_safe_to_string(context_, -1);
 
         duk_pop(context_);
+
+        if (!error.empty())
+            throw plugin_error(plugin_error::exec_error, std::move(error));
     }
 }
 
--- a/libirccd-test/irccd/command_test.hpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd-test/irccd/command_test.hpp	Tue Nov 28 17:17:30 2017 +0100
@@ -21,6 +21,8 @@
 
 #include <memory>
 
+#include <irccd/logger.hpp>
+
 #include <irccd/irccd.hpp>
 #include <irccd/command_service.hpp>
 #include <irccd/transport_service.hpp>
@@ -62,6 +64,8 @@
 {
     using boost::asio::ip::tcp;
 
+    log::set_logger(std::make_unique<log::silent_logger>());
+
     // Bind to a random port.
     tcp::endpoint ep(tcp::v4(), 0);
     tcp::acceptor acc(service_, ep);
--- a/libirccd/irccd/command.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/command.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -208,7 +208,7 @@
 
 void plugin_reload_command::exec(irccd& irccd, transport_client& client, const nlohmann::json& args)
 {
-    irccd.plugins().require(json_util::require_identifier(args, "plugin"))->on_reload(irccd);
+    irccd.plugins().reload(json_util::require_identifier(args, "plugin"));
     client.success("plugin-reload");
 }
 
--- a/libirccd/irccd/command.hpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/command.hpp	Tue Nov 28 17:17:30 2017 +0100
@@ -90,6 +90,10 @@
 
 /**
  * \brief Implementation of plugin-config transport command.
+ *
+ * Replies:
+ *
+ *   - plugin_error::not_found
  */
 class plugin_config_command : public command {
 public:
@@ -106,6 +110,10 @@
 
 /**
  * \brief Implementation of plugin-info transport command.
+ *
+ * Replies:
+ *
+ *   - plugin_error::not_found
  */
 class plugin_info_command : public command {
 public:
@@ -138,6 +146,12 @@
 
 /**
  * \brief Implementation of plugin-load transport command.
+ *
+ * Replies:
+ *
+ *   - plugin_error::already_exists
+ *   - plugin_error::not_found
+ *   - pluign_error::exec_error
  */
 class plugin_load_command : public command {
 public:
@@ -154,6 +168,11 @@
 
 /**
  * \brief Implementation of plugin-reload transport command.
+ *
+ * Replies:
+ *
+ *   - plugin_error::not_found
+ *   - pluign_error::exec_error
  */
 class plugin_reload_command : public command {
 public:
@@ -170,6 +189,11 @@
 
 /**
  * \brief Implementation of plugin-unload transport command.
+ *
+ * Replies:
+ *
+ *   - plugin_error::not_found
+ *   - pluign_error::exec_error
  */
 class plugin_unload_command : public command {
 public:
--- a/libirccd/irccd/plugin_service.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/plugin_service.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -47,8 +47,13 @@
 
 plugin_service::~plugin_service()
 {
-    for (const auto& plugin : plugins_)
-        plugin->on_unload(irccd_);
+    for (const auto& plugin : plugins_) {
+        try {
+            plugin->on_unload(irccd_);
+        } catch (const std::exception& ex) {
+            log::warning() << "plugin: " << plugin->name() << ": " << ex.what() << std::endl;
+        }
+    }
 }
 
 bool plugin_service::has(const std::string& name) const noexcept
@@ -152,7 +157,7 @@
 void plugin_service::load(std::string name, std::string path)
 {
     if (has(name))
-        return;
+        throw plugin_error(plugin_error::already_exists);
 
     std::shared_ptr<plugin> plugin;
 
@@ -167,8 +172,8 @@
     plugin->set_config(config(name));
     plugin->set_formats(formats(name));
     plugin->set_paths(paths(name));
-    plugin->on_load(irccd_);
 
+    exec(plugin, &plugin::on_load, irccd_);
     add(std::move(plugin));
 }
 
@@ -176,8 +181,10 @@
 {
     auto plugin = get(name);
 
-    if (plugin)
-        plugin->on_reload(irccd_);
+    if (!plugin)
+        throw plugin_error(plugin_error::not_found);
+
+    exec(plugin, &plugin::on_reload, irccd_);
 }
 
 void plugin_service::unload(const std::string& name)
@@ -186,10 +193,14 @@
         return plugin->name() == name;
     });
 
-    if (it != plugins_.end()) {
-        (*it)->on_unload(irccd_);
-        plugins_.erase(it);
-    }
+    if (it == plugins_.end())
+        throw plugin_error(plugin_error::not_found);
+
+    // Erase first, in case of throwing.
+    auto save = *it;
+
+    plugins_.erase(it);
+    exec(save, &plugin::on_unload, irccd_);
 }
 
 } // !irccd
--- a/libirccd/irccd/plugin_service.hpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/plugin_service.hpp	Tue Nov 28 17:17:30 2017 +0100
@@ -24,6 +24,7 @@
  * \brief Plugin service.
  */
 
+#include <cassert>
 #include <memory>
 #include <string>
 #include <vector>
@@ -177,6 +178,50 @@
      * \throw std::exception on failures
      */
     void reload(const std::string& name);
+
+    /**
+     * Call a plugin function and throw an exception with the following errors:
+     *
+     *   - plugin_error::not_found if not loaded
+     *   - plugin_error::exec_error if function failed
+     *
+     * \pre plugin != nullptr
+     * \param plugin the plugin
+     * \param func the plugin member function (pointer to member)
+     * \param args the arguments to pass
+     */
+    template <typename Func, typename... Args>
+    void exec(std::shared_ptr<plugin> plugin, Func fn, Args&&... args)
+    {
+        assert(plugin);
+
+        // TODO: replace with C++17 std::invoke.
+        try {
+            ((*plugin).*(fn))(std::forward<Args>(args)...);
+        } catch (const std::exception& ex) {
+            throw plugin_error(plugin_error::exec_error, ex.what());
+        } catch (...) {
+            throw plugin_error(plugin_error::exec_error);
+        }
+    }
+
+    /**
+     * Overloaded function.
+     *
+     * \param name the plugin name
+     * \param func the plugin member function (pointer to member)
+     * \param args the arguments to pass
+     */
+    template <typename Func, typename... Args>
+    void exec(const std::string& name, Func fn, Args&&... args)
+    {
+        auto plugin = find(name);
+
+        if (!plugin)
+            throw plugin_error(plugin_error::not_found);
+
+        exec(plugin, fn, std::forward<Args>(args)...);
+    }
 };
 
 } // !irccd
--- a/libirccd/irccd/rule.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/rule.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -68,8 +68,10 @@
         std::string message(int e) const override
         {
             switch (static_cast<rule_error::error>(e)) {
-            case rule_error::error::invalid_action:
+            case rule_error::invalid_action:
                 return "invalid action given";
+            case rule_error::invalid_index:
+                return "invalid index";
             default:
                 return "no error";
             }
--- a/libirccd/irccd/rule.hpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/rule.hpp	Tue Nov 28 17:17:30 2017 +0100
@@ -222,7 +222,7 @@
     /**
      * \brief Rule related errors (4000..4999)
      */
-    enum class error {
+    enum error {
         //!< No error.
         no_error = 0,
 
--- a/libirccd/irccd/rule_service.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/libirccd/irccd/rule_service.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -44,7 +44,7 @@
 const rule &rule_service::require(unsigned position) const
 {
     if (position >= rules_.size())
-        throw std::out_of_range("rule " + std::to_string(position) + " does not exist");
+        throw rule_error(rule_error::invalid_index);
 
     return rules_[position];
 }
@@ -52,7 +52,7 @@
 rule &rule_service::require(unsigned position)
 {
     if (position >= rules_.size())
-        throw std::out_of_range("rule " + std::to_string(position) + " does not exist");
+        throw rule_error(rule_error::invalid_index);
 
     return rules_[position];
 }
--- a/tests/cmd-plugin-config/main.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/tests/cmd-plugin-config/main.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -129,6 +129,29 @@
     BOOST_TEST(json["variables"]["x2"].get<std::string>() == "20");
 }
 
+BOOST_AUTO_TEST_SUITE(errors)
+
+BOOST_AUTO_TEST_CASE(not_found)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-config" },
+        { "plugin",     "unknown"       }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::not_found);
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
 BOOST_AUTO_TEST_SUITE_END()
 
 } // !irccd
--- a/tests/cmd-plugin-info/main.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/tests/cmd-plugin-info/main.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -58,28 +58,29 @@
     BOOST_TEST(response["version"].get<std::string>() == "0.0.0.0.0.0.0.0.1-beta5");
 }
 
-BOOST_AUTO_TEST_CASE(notfound)
+BOOST_AUTO_TEST_SUITE(errors)
+
+BOOST_AUTO_TEST_CASE(not_found)
 {
-    auto response = nlohmann::json();
+    boost::system::error_code result;
 
-    ctl_->recv([&] (auto, auto msg) {
-        response = std::move(msg);
+    ctl_->send({
+        { "command",    "plugin-info"   },
+        { "plugin",     "unknown"       }
     });
-    ctl_->send({
-        { "command",    "plugin-info"       },
-        { "plugin",     "test"              },
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
     });
 
-    wait_for([&] () {
-        return response.is_object();
+    wait_for([&] {
+        return result;
     });
 
-    BOOST_TEST(response.is_object());
-
-    // TODO: error code here.
-    BOOST_TEST(response["error"].get<std::string>() == "plugin test not found");
+    BOOST_ASSERT(result == plugin_error::not_found);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
 
+BOOST_AUTO_TEST_SUITE_END()
+
 } // !irccd
--- a/tests/cmd-plugin-load/main.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/tests/cmd-plugin-load/main.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -38,7 +38,26 @@
 
     std::shared_ptr<plugin> find(const std::string& id) noexcept override
     {
-        return std::make_unique<plugin>(id, "");
+        class broken : public plugin {
+        public:
+            using plugin::plugin;
+
+            void on_load(irccd&) override
+            {
+                throw std::runtime_error("broken");
+            }
+        };
+
+        /*
+         * The 'magic' plugin will be created for the unit tests, all other
+         * plugins will return null.
+         */
+        if (id == "magic")
+            return std::make_unique<plugin>(id, "");
+        if (id == "broken")
+            return std::make_unique<broken>(id, "");
+
+        return nullptr;
     }
 };
 
@@ -47,28 +66,90 @@
     plugin_load_test()
     {
         daemon_->plugins().add_loader(std::make_unique<custom_loader>());
+        daemon_->plugins().add(std::make_unique<plugin>("already", ""));
     }
 };
 
-} // !irccd
+} // !namespace
 
 BOOST_FIXTURE_TEST_SUITE(plugin_load_test_suite, plugin_load_test)
 
 BOOST_AUTO_TEST_CASE(basic)
 {
     ctl_->send({
-        { "command", "plugin-load" },
-        { "plugin", "foo" }
+        { "command",    "plugin-load"   },
+        { "plugin",     "magic"         }
     });
 
     wait_for([&] () {
-        return daemon_->plugins().has("foo");
+        return daemon_->plugins().has("magic");
     });
 
     BOOST_TEST(!daemon_->plugins().list().empty());
-    BOOST_TEST(daemon_->plugins().has("foo"));
+    BOOST_TEST(daemon_->plugins().has("magic"));
+}
+
+BOOST_AUTO_TEST_SUITE(errors)
+
+BOOST_AUTO_TEST_CASE(not_found)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-load"   },
+        { "plugin",     "unknown"       }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::not_found);
+}
+
+BOOST_AUTO_TEST_CASE(already_exists)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-load"   },
+        { "plugin",     "already"       }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::already_exists);
+}
+
+BOOST_AUTO_TEST_CASE(exec_error)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-load"   },
+        { "plugin",     "broken"        }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::exec_error);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
 
+BOOST_AUTO_TEST_SUITE_END()
+
 } // !irccd
--- a/tests/cmd-plugin-reload/main.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/tests/cmd-plugin-reload/main.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -43,6 +43,16 @@
     }
 };
 
+class broken_plugin : public plugin {
+public:
+    using plugin::plugin;
+
+    void on_reload(irccd&) override
+    {
+        throw std::runtime_error("broken");
+    }
+};
+
 class plugin_reload_test : public command_test<plugin_reload_command> {
 protected:
     std::shared_ptr<custom_plugin> plugin_;
@@ -51,6 +61,7 @@
         : plugin_(std::make_shared<custom_plugin>())
     {
         daemon_->plugins().add(plugin_);
+        daemon_->plugins().add(std::make_unique<broken_plugin>("broken", ""));
     }
 };
 
@@ -72,27 +83,48 @@
     BOOST_TEST(plugin_->reloaded);
 }
 
-BOOST_AUTO_TEST_CASE(notfound)
-{
-    auto response = nlohmann::json();
+BOOST_AUTO_TEST_SUITE(errors)
 
-    ctl_->recv([&] (auto, auto msg) {
-        response = msg;
-    });
+BOOST_AUTO_TEST_CASE(not_found)
+{
+    boost::system::error_code result;
+
     ctl_->send({
         { "command",    "plugin-reload" },
-        { "plugin",     "no"            }
+        { "plugin",     "unknown"       }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
     });
 
-    wait_for([&] () {
-        return response.is_object();
+    BOOST_ASSERT(result == plugin_error::not_found);
+}
+
+BOOST_AUTO_TEST_CASE(exec_error)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-reload" },
+        { "plugin",     "broken"        }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
     });
 
-    // TODO: error code
-    BOOST_TEST(response.is_object());
-    BOOST_TEST(response["error"].get<std::string>() == "plugin no not found");
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::exec_error);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
 
+BOOST_AUTO_TEST_SUITE_END()
+
 } // !irccd
--- a/tests/cmd-plugin-unload/main.cpp	Tue Nov 28 13:57:09 2017 +0100
+++ b/tests/cmd-plugin-unload/main.cpp	Tue Nov 28 17:17:30 2017 +0100
@@ -43,6 +43,16 @@
     }
 };
 
+class broken_plugin : public plugin {
+public:
+    using plugin::plugin;
+
+    void on_unload(irccd&) override
+    {
+        throw std::runtime_error("broken");
+    }
+};
+
 class plugin_unload_test : public command_test<plugin_unload_command> {
 protected:
     std::shared_ptr<custom_plugin> plugin_;
@@ -51,6 +61,7 @@
         : plugin_(std::make_shared<custom_plugin>())
     {
         daemon_->plugins().add(plugin_);
+        daemon_->plugins().add(std::make_unique<broken_plugin>("broken", ""));
     }
 };
 
@@ -72,6 +83,49 @@
     BOOST_TEST(plugin_->unloaded);
 }
 
+BOOST_AUTO_TEST_SUITE(errors)
+
+BOOST_AUTO_TEST_CASE(not_found)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-unload" },
+        { "plugin",     "unknown"       }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::not_found);
+}
+
+BOOST_AUTO_TEST_CASE(exec_error)
+{
+    boost::system::error_code result;
+
+    ctl_->send({
+        { "command",    "plugin-unload" },
+        { "plugin",     "broken"        }
+    });
+    ctl_->recv([&] (auto code, auto) {
+        result = code;
+    });
+
+    wait_for([&] {
+        return result;
+    });
+
+    BOOST_ASSERT(result == plugin_error::exec_error);
+    BOOST_ASSERT(!daemon_->plugins().has("broken"));
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
 BOOST_AUTO_TEST_SUITE_END()
 
 } // !irccd