# HG changeset patch # User David Demelier # Date 1511885850 -3600 # Node ID 153e84e7b09b353d8518190246b66fa358c3d948 # Parent 24b79bccc18120726a24634ea1b9c2a01fe6644a Tests: add error code support in cmd-plugin-* tests diff -r 24b79bccc181 -r 153e84e7b09b libirccd-js/irccd/js/js_plugin.cpp --- 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)); } } diff -r 24b79bccc181 -r 153e84e7b09b libirccd-test/irccd/command_test.hpp --- 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 +#include + #include #include #include @@ -62,6 +64,8 @@ { using boost::asio::ip::tcp; + log::set_logger(std::make_unique()); + // Bind to a random port. tcp::endpoint ep(tcp::v4(), 0); tcp::acceptor acc(service_, ep); diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/command.cpp --- 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"); } diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/command.hpp --- 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: diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/plugin_service.cpp --- 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; @@ -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 diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/plugin_service.hpp --- 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 #include #include #include @@ -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 + void exec(std::shared_ptr plugin, Func fn, Args&&... args) + { + assert(plugin); + + // TODO: replace with C++17 std::invoke. + try { + ((*plugin).*(fn))(std::forward(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 + 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)...); + } }; } // !irccd diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/rule.cpp --- 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(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"; } diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/rule.hpp --- 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, diff -r 24b79bccc181 -r 153e84e7b09b libirccd/irccd/rule_service.cpp --- 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]; } diff -r 24b79bccc181 -r 153e84e7b09b tests/cmd-plugin-config/main.cpp --- 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() == "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 diff -r 24b79bccc181 -r 153e84e7b09b tests/cmd-plugin-info/main.cpp --- 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() == "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() == "plugin test not found"); + BOOST_ASSERT(result == plugin_error::not_found); } BOOST_AUTO_TEST_SUITE_END() +BOOST_AUTO_TEST_SUITE_END() + } // !irccd diff -r 24b79bccc181 -r 153e84e7b09b tests/cmd-plugin-load/main.cpp --- 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 find(const std::string& id) noexcept override { - return std::make_unique(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(id, ""); + if (id == "broken") + return std::make_unique(id, ""); + + return nullptr; } }; @@ -47,28 +66,90 @@ plugin_load_test() { daemon_->plugins().add_loader(std::make_unique()); + daemon_->plugins().add(std::make_unique("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 diff -r 24b79bccc181 -r 153e84e7b09b tests/cmd-plugin-reload/main.cpp --- 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 { protected: std::shared_ptr plugin_; @@ -51,6 +61,7 @@ : plugin_(std::make_shared()) { daemon_->plugins().add(plugin_); + daemon_->plugins().add(std::make_unique("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() == "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 diff -r 24b79bccc181 -r 153e84e7b09b tests/cmd-plugin-unload/main.cpp --- 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 { protected: std::shared_ptr plugin_; @@ -51,6 +61,7 @@ : plugin_(std::make_shared()) { daemon_->plugins().add(plugin_); + daemon_->plugins().add(std::make_unique("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