From 6e784c916233de956179babd9b1904b1313665ee Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 14 Oct 2024 15:59:42 +0200 Subject: [PATCH 1/2] WIP: exceptions during command calls / handling Signed-off-by: Kai-Uwe Hermann --- include/framework/everest.hpp | 11 ++++++++++ lib/everest.cpp | 39 ++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/include/framework/everest.hpp b/include/framework/everest.hpp index b0f7252f..b4c9855f 100644 --- a/include/framework/everest.hpp +++ b/include/framework/everest.hpp @@ -33,6 +33,17 @@ using TelemetryEntry = std::variant; using UnsubscribeToken = std::function; +/// \brief Result of a command +struct CmdResult { + std::optional result; + std::optional error; // TODO: use proper ErrorType instead of json +}; + +class EverestCmdError : public Everest::EverestBaseRuntimeError { +public: + using EverestBaseRuntimeError::EverestBaseRuntimeError; +}; + namespace error { struct ErrorDatabaseMap; struct ErrorManagerImpl; diff --git a/lib/everest.cpp b/lib/everest.cpp index 8d870dc6..c9e560ff 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -354,8 +354,8 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json std::string call_id = boost::uuids::to_string(boost::uuids::random_generator()()); - std::promise res_promise; - std::future res_future = res_promise.get_future(); + std::promise res_promise; + std::future res_future = res_promise.get_future(); Handler res_handler = [this, &res_promise, call_id, connection, cmd_name, return_type](json data) { auto& data_id = data.at("id"); @@ -368,7 +368,14 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json "Incoming res {} for {}->{}()", data_id, this->config.printable_identifier(connection["module_id"], connection["implementation_id"]), cmd_name); - res_promise.set_value(std::move(data["retval"])); + if (data.contains("error")) { + EVLOG_error << fmt::format( + "Received error {} for {}->{}()", data.at("error"), + this->config.printable_identifier(connection["module_id"], connection["implementation_id"]), cmd_name); + res_promise.set_value({std::nullopt, data.at("error")}); + } else { + res_promise.set_value({std::move(data["retval"]), std::nullopt}); + } }; const auto cmd_topic = @@ -393,7 +400,7 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json res_future_status = res_future.wait_until(res_wait); } while (res_future_status == std::future_status::deferred); - json result; + CmdResult result; if (res_future_status == std::future_status::timeout) { EVLOG_AND_THROW(EverestTimeoutError(fmt::format( "Timeout while waiting for result of {}->{}()", @@ -404,7 +411,16 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json } this->mqtt_abstraction.unregister_handler(cmd_topic, res_token); - return result; + if (result.error.has_value()) { + // throw appropriate exception + auto& error = result.error.value(); + auto error_message = fmt::format("{}: {}", error.at("type"), error.at("msg")); + throw EverestBaseRuntimeError(error_message); + } else if (not result.result.has_value()) { + throw EverestBaseRuntimeError("Command did not return result"); + } else { + return result.result.value(); + } } void Everest::publish_var(const std::string& impl_id, const std::string& var_name, json value) { @@ -872,12 +888,20 @@ void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, // publish results json res_data = json({}); res_data["id"] = data["id"]; + auto error = false; // call real cmd handler - res_data["retval"] = handler(data["args"]); + try { + res_data["retval"] = handler(data["args"]); + } catch (const std::exception& e) { + EVLOG_verbose << fmt::format("Exception during handling of: {}->{}({}): {}", + this->config.printable_identifier(this->module_id, impl_id), cmd_name, + fmt::join(arg_names, ","), e.what()); + res_data["error"] = {{"type", "HandlerException"}, {"msg", e.what()}}; + } // check retval agains manifest - if (this->validate_data_with_schema) { + if (not error && this->validate_data_with_schema) { try { // only use validator on non-null return types if (!(res_data["retval"].is_null() && @@ -897,7 +921,6 @@ void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, } } - EVLOG_verbose << fmt::format("RETVAL: {}", res_data["retval"].dump()); res_data["origin"] = this->module_id; json res_publish_data = json::object({{"name", cmd_name}, {"type", "result"}, {"data", res_data}}); From 2f80665f75f2b952671b56523c46a5318edaebb1 Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Thu, 24 Oct 2024 12:54:11 +0200 Subject: [PATCH 2/2] WIP: Use ErrorType in CmdResult Signed-off-by: Kai-Uwe Hermann --- include/framework/everest.hpp | 28 ++++++++++- lib/everest.cpp | 91 +++++++++++++++++++++++++++++------ 2 files changed, 104 insertions(+), 15 deletions(-) diff --git a/include/framework/everest.hpp b/include/framework/everest.hpp index b4c9855f..c56b180d 100644 --- a/include/framework/everest.hpp +++ b/include/framework/everest.hpp @@ -33,10 +33,36 @@ using TelemetryEntry = std::variant; using UnsubscribeToken = std::function; +enum class ErrorType { + MessageParsing, + SchemaValidation, + HandlerException, + Timeout, + Shutdown, + Unknown +}; + +struct ErrorMessage { + ErrorType type; + std::string msg; +}; + +namespace conversions { +std::string error_type_to_string(ErrorType error_type); +ErrorType string_to_error_type(const std::string& error_type_string); +} // namespace conversions + +void to_json(nlohmann::json& j, const ErrorMessage& e); +void from_json(const nlohmann::json& j, ErrorMessage& e); + /// \brief Result of a command struct CmdResult { std::optional result; - std::optional error; // TODO: use proper ErrorType instead of json + std::optional error; + + // CmdResult(std::optional result = std::nullopt, std::optional error = std::nullopt) : + // result(result), error(error) { + // } }; class EverestCmdError : public Everest::EverestBaseRuntimeError { diff --git a/lib/everest.cpp b/lib/everest.cpp index c9e560ff..4f5f9b0d 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -32,6 +32,54 @@ namespace Everest { const auto remote_cmd_res_timeout_seconds = 300; const std::array TELEMETRY_RESERVED_KEYS = {{"connector_id"}}; +namespace conversions { +constexpr auto ERROR_TYPE_MESSAGE_PARSING = "MessageParsing"; +constexpr auto ERROR_TYPE_SCHEMA_VALIDATION = "SchemaValidation"; +constexpr auto ERROR_TYPE_HANDLER_EXCEPTION = "HandlerException"; +constexpr auto ERROR_TYPE_TIMEOUT = "Timeout"; +constexpr auto ERROR_TYPE_SHUTDOWN = "Shutdown"; +constexpr auto ERROR_TYPE_UNKNOWN = "Unknown"; +std::string error_type_to_string(ErrorType error_type) { + switch (error_type) { + case ErrorType::MessageParsing: + return ERROR_TYPE_MESSAGE_PARSING; + break; + case ErrorType::SchemaValidation: + return ERROR_TYPE_SCHEMA_VALIDATION; + break; + case ErrorType::HandlerException: + return ERROR_TYPE_HANDLER_EXCEPTION; + break; + case ErrorType::Timeout: + return ERROR_TYPE_TIMEOUT; + break; + case ErrorType::Shutdown: + return ERROR_TYPE_SHUTDOWN; + break; + case ErrorType::Unknown: + return ERROR_TYPE_UNKNOWN; + break; + } + + return ERROR_TYPE_UNKNOWN; +} +ErrorType string_to_error_type(const std::string& error_type_string) { + if (error_type_string == ERROR_TYPE_MESSAGE_PARSING) { + return ErrorType::MessageParsing; + } else if (error_type_string == ERROR_TYPE_SCHEMA_VALIDATION) { + return ErrorType::SchemaValidation; + } else if (error_type_string == ERROR_TYPE_HANDLER_EXCEPTION) { + return ErrorType::HandlerException; + } else if (error_type_string == ERROR_TYPE_TIMEOUT) { + return ErrorType::Timeout; + } else if (error_type_string == ERROR_TYPE_SHUTDOWN) { + return ErrorType::Shutdown; + } + + return ErrorType::Unknown; +} +} // namespace conversions + Everest::Everest(std::string module_id_, const Config& config_, bool validate_data_with_schema, const std::string& mqtt_server_socket_path, const std::string& mqtt_server_address, int mqtt_server_port, const std::string& mqtt_everest_prefix, const std::string& mqtt_external_prefix, @@ -372,9 +420,9 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json EVLOG_error << fmt::format( "Received error {} for {}->{}()", data.at("error"), this->config.printable_identifier(connection["module_id"], connection["implementation_id"]), cmd_name); - res_promise.set_value({std::nullopt, data.at("error")}); + res_promise.set_value(CmdResult{std::nullopt, data.at("error")}); } else { - res_promise.set_value({std::move(data["retval"]), std::nullopt}); + res_promise.set_value(CmdResult{std::move(data["retval"]), std::nullopt}); } }; @@ -412,12 +460,10 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json this->mqtt_abstraction.unregister_handler(cmd_topic, res_token); if (result.error.has_value()) { - // throw appropriate exception auto& error = result.error.value(); - auto error_message = fmt::format("{}: {}", error.at("type"), error.at("msg")); - throw EverestBaseRuntimeError(error_message); + throw EverestCmdError(fmt::format("{}: {}", conversions::error_type_to_string(error.type), error.msg)); } else if (not result.result.has_value()) { - throw EverestBaseRuntimeError("Command did not return result"); + throw EverestCmdError("Command did not return result"); } else { return result.result.value(); } @@ -862,6 +908,11 @@ void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, this->config.printable_identifier(this->module_id, impl_id), cmd_name, fmt::join(arg_names, ",")); + json res_data = json({}); + // FIXME: this id lookup might fail -> return MessageParsing error + res_data["id"] = data["id"]; + std::optional error; + // check data and ignore it if not matching (publishing it should have // been prohibited already) if (this->validate_data_with_schema) { @@ -881,27 +932,26 @@ void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, } catch (const std::exception& e) { EVLOG_warning << fmt::format("Ignoring incoming cmd '{}' because not matching manifest schema: {}", cmd_name, e.what()); - return; + error = ErrorMessage{ErrorType::SchemaValidation, e.what()}; } } // publish results - json res_data = json({}); - res_data["id"] = data["id"]; - auto error = false; // call real cmd handler try { - res_data["retval"] = handler(data["args"]); + if (not error.has_value()) { + res_data["retval"] = handler(data["args"]); + } } catch (const std::exception& e) { EVLOG_verbose << fmt::format("Exception during handling of: {}->{}({}): {}", this->config.printable_identifier(this->module_id, impl_id), cmd_name, fmt::join(arg_names, ","), e.what()); - res_data["error"] = {{"type", "HandlerException"}, {"msg", e.what()}}; + error = ErrorMessage{ErrorType::HandlerException, e.what()}; } // check retval agains manifest - if (not error && this->validate_data_with_schema) { + if (not error.has_value() && this->validate_data_with_schema) { try { // only use validator on non-null return types if (!(res_data["retval"].is_null() && @@ -917,11 +967,14 @@ void Everest::provide_cmd(const std::string impl_id, const std::string cmd_name, EVLOG_warning << fmt::format("Ignoring return value of cmd '{}' because the validation of the result " "failed: {}\ndefinition: {}\ndata: {}", cmd_name, e.what(), cmd_definition, res_data); - return; + error = ErrorMessage{ErrorType::SchemaValidation, e.what()}; } } res_data["origin"] = this->module_id; + if (error.has_value()) { + res_data["error"] = error.value(); + } json res_publish_data = json::object({{"name", cmd_name}, {"type", "result"}, {"data", res_data}}); @@ -1099,4 +1152,14 @@ bool Everest::check_arg(ArgumentType arg_types, json manifest_arg) { } return true; } + +// TODO fix these conversions +void to_json(nlohmann::json& j, const ErrorMessage& e) { + j = {{"type", conversions::error_type_to_string(e.type)}, {"msg", e.msg}}; +} + +void from_json(const nlohmann::json& j, ErrorMessage& e) { + e.type = conversions::string_to_error_type(j.at("type")); + e.msg = j.at("msg"); +} } // namespace Everest