From 7bc16193886a848152205a923be1728530321686 Mon Sep 17 00:00:00 2001 From: "Craig Edwards (Brain)" Date: Wed, 4 Oct 2023 18:14:59 +0100 Subject: [PATCH] feat: improved human readable error messages to default logger, with unit test (#921) --- .vscode/settings.json | 3 +- include/dpp/restresults.h | 8 +++ src/dpp/cluster/confirmation.cpp | 74 +++++++++++++++++++++---- src/dpp/utility.cpp | 4 +- src/unittest/test.cpp | 93 ++++++++++++++++++++++++++++++++ src/unittest/test.h | 1 + 6 files changed, 171 insertions(+), 12 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index f00d7bf8ee..21ab674e7d 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -79,6 +79,7 @@ "coroutine": "cpp", "numbers": "cpp", "semaphore": "cpp", - "stop_token": "cpp" + "stop_token": "cpp", + "charconv": "cpp" } } \ No newline at end of file diff --git a/include/dpp/restresults.h b/include/dpp/restresults.h index 0691b41869..d6ad8bfa57 100644 --- a/include/dpp/restresults.h +++ b/include/dpp/restresults.h @@ -216,6 +216,10 @@ struct DPP_EXPORT error_detail { * @brief Error reason (full message) */ std::string reason; + /** + * @brief Object field index + */ + int index = 0; }; /** @@ -235,6 +239,10 @@ struct DPP_EXPORT error_info { * @brief Field specific error descriptions */ std::vector errors; + /** + * @brief Human readable error message constructed from the above + */ + std::string human_readable; }; /** diff --git a/src/dpp/cluster/confirmation.cpp b/src/dpp/cluster/confirmation.cpp index b2744f8d8f..41cc6915a1 100644 --- a/src/dpp/cluster/confirmation.cpp +++ b/src/dpp/cluster/confirmation.cpp @@ -82,34 +82,55 @@ error_info confirmation_callback_t::get_error() const { json& errors = j["errors"]; for (auto obj = errors.begin(); obj != errors.end(); ++obj) { - if (obj->find("0") != obj->end()) { + /* Arrays in the error report are numerically indexed with a number in a string. Ugh. */ + if (isdigit(*(obj.key().c_str()))) { /* An array of error messages */ + int array_index = std::atoll(obj.key().c_str()); for (auto index = obj->begin(); index != obj->end(); ++index) { if (index->find("_errors") != index->end()) { + /* A single object where one or more fields generated an error */ for (auto errordetails = (*index)["_errors"].begin(); errordetails != (*index)["_errors"].end(); ++errordetails) { error_detail detail; detail.code = (*errordetails)["code"].get(); detail.reason = (*errordetails)["message"].get(); detail.object.clear(); detail.field = obj.key(); + detail.index = array_index; e.errors.emplace_back(detail); } } else { + /* An object where one or more fields within it generated an error, e.g. slash command */ for (auto fields = index->begin(); fields != index->end(); ++fields) { - for (auto errordetails = (*fields)["_errors"].begin(); errordetails != (*fields)["_errors"].end(); ++errordetails) { - error_detail detail; - detail.code = (*errordetails)["code"].get(); - detail.reason = (*errordetails)["message"].get(); - detail.field = fields.key(); - detail.object = obj.key(); - e.errors.emplace_back(detail); + if (fields->find("_errors") != fields->end()) { + for (auto errordetails = (*fields)["_errors"].begin(); errordetails != (*fields)["_errors"].end(); ++errordetails) { + error_detail detail; + detail.code = (*errordetails)["code"].get(); + detail.reason = (*errordetails)["message"].get(); + detail.field = fields.key(); + detail.object = obj.key(); + detail.index = array_index; + e.errors.emplace_back(detail); + } + } else { + /* An array of objects where one or more generated an error, e.g. slash command bulk registration */ + for (auto fields2 = fields->begin(); fields2 != fields->end(); ++fields2) { + for (auto errordetails = (*fields2)["_errors"].begin(); errordetails != (*fields2)["_errors"].end(); ++errordetails) { + error_detail detail; + detail.code = (*errordetails)["code"].get(); + detail.reason = (*errordetails)["message"].get(); + detail.field = index.key() + "[" + fields.key() + "]." + fields2.key(); + detail.object = obj.key(); + detail.index = array_index; + e.errors.emplace_back(detail); + } + } } } } } } else if (obj->find("_errors") != obj->end()) { - /* An object of error messages */ + /* An object of error messages (rare) */ e.errors.reserve((*obj)["_errors"].size()); for (auto errordetails = (*obj)["_errors"].begin(); errordetails != (*obj)["_errors"].end(); ++errordetails) { error_detail detail; @@ -117,8 +138,43 @@ error_info confirmation_callback_t::get_error() const { detail.reason = (*errordetails)["message"].get(); detail.object.clear(); detail.field = obj.key(); + detail.index = 0; e.errors.emplace_back(detail); } + } else { + /* An object that has a subobject with errors */ + for (auto index = obj->begin(); index != obj->end(); ++index) { + int array_index = std::atoll(index.key().c_str()); + for (auto index2 = index->begin(); index2 != index->end(); ++index2) { + if (index2->find("_errors") != index2->end()) { + /* A single object where one or more fields generated an error */ + for (auto errordetails = (*index2)["_errors"].begin(); errordetails != (*index2)["_errors"].end(); ++errordetails) { + error_detail detail; + detail.code = (*errordetails)["code"].get(); + detail.reason = (*errordetails)["message"].get(); + detail.object = obj.key(); + detail.field = index2.key(); + detail.index = array_index; + e.errors.emplace_back(detail); + } + } + } + } + } + } + + e.human_readable = std::to_string(e.code) + ": " + e.message; + std::string prefix = e.errors.size() == 1 ? " " : "\n\t"; + for (const auto& error : e.errors) { + if (error.object.empty()) { + /* A singular field with an error in an unnamed object */ + e.human_readable += prefix + "- " + error.field + ": " + error.reason + " (" + error.code + ")"; + } else if (isdigit(*(error.object.c_str()))) { + /* An unnamed array of objects where one or more generated an error, e.g. slash command bulk registration */ + e.human_readable += prefix + "- [" + error.object + "]." + error.field + ": " + error.reason + " (" + error.code + ")"; + } else { + /* A named array of objects whre a field in the object has an error */ + e.human_readable += prefix + "- " + error.object + "[" + std::to_string(error.index) + "]." + error.field + ": " + error.reason + " (" + error.code + ")"; } } diff --git a/src/dpp/utility.cpp b/src/dpp/utility.cpp index 3c526df04c..1ca4751ca3 100644 --- a/src/dpp/utility.cpp +++ b/src/dpp/utility.cpp @@ -499,10 +499,10 @@ namespace dpp { return [](const dpp::confirmation_callback_t& detail) { if (detail.is_error()) { if (detail.bot) { + error_info e = detail.get_error(); detail.bot->log( dpp::ll_error, - "Error " + std::to_string(detail.get_error().code) + " [" + - detail.get_error().message + "] on API request, returned content was: " + detail.http_info.body + "Error: " + e.human_readable ); } } diff --git a/src/unittest/test.cpp b/src/unittest/test.cpp index 8173c6842c..9c82ee7863 100644 --- a/src/unittest/test.cpp +++ b/src/unittest/test.cpp @@ -58,6 +58,99 @@ Markdown lol ||spoiler|| ~~strikethrough~~ `small *code* block`\n"; set_test(COMPARISON, u1 == u2 && u1 != u3); + set_test(ERRORS, false); + + /* Prepare a confirmation_callback_t in error state (400) */ + dpp::confirmation_callback_t error_test; + bool error_message_success = false; + error_test.http_info.status = 400; + + error_test.http_info.body = "{\ + \"message\": \"Invalid Form Body\",\ + \"code\": 50035,\ + \"errors\": {\ + \"options\": {\ + \"0\": {\ + \"name\": {\ + \"_errors\": [\ + {\ + \"code\": \"STRING_TYPE_REGEX\",\ + \"message\": \"String value did not match validation regex.\"\ + },\ + {\ + \"code\": \"APPLICATION_COMMAND_INVALID_NAME\",\ + \"message\": \"Command name is invalid\"\ + }\ + ]\ + }\ + }\ + }\ + }\ + }"; + error_message_success = (error_test.get_error().human_readable == "50035: Invalid Form Body\n\t- options[0].name: String value did not match validation regex. (STRING_TYPE_REGEX)\n\t- options[0].name: Command name is invalid (APPLICATION_COMMAND_INVALID_NAME)"); + + error_test.http_info.body = "{\ + \"message\": \"Invalid Form Body\",\ + \"code\": 50035,\ + \"errors\": {\ + \"type\": {\ + \"_errors\": [\ + {\ + \"code\": \"BASE_TYPE_CHOICES\",\ + \"message\": \"Value must be one of {4, 5, 9, 10, 11}.\"\ + }\ + ]\ + }\ + }\ + }"; + error_message_success = (error_message_success && error_test.get_error().human_readable == "50035: Invalid Form Body - type: Value must be one of {4, 5, 9, 10, 11}. (BASE_TYPE_CHOICES)"); + + error_test.http_info.body = "{\ + \"message\": \"Unknown Guild\",\ + \"code\": 10004\ + }"; + error_message_success = (error_message_success && error_test.get_error().human_readable == "10004: Unknown Guild"); + + error_test.http_info.body = "{\ + \"message\": \"Invalid Form Body\",\ + \"code\": 50035,\ + \"errors\": {\ + \"allowed_mentions\": {\ + \"_errors\": [\ + {\ + \"code\": \"MESSAGE_ALLOWED_MENTIONS_PARSE_EXCLUSIVE\",\ + \"message\": \"parse:[\\\"users\\\"] and users: [ids...] are mutually exclusive.\"\ + }\ + ]\ + }\ + }\ + }"; + error_message_success = (error_message_success && error_test.get_error().human_readable == "50035: Invalid Form Body - allowed_mentions: parse:[\"users\"] and users: [ids...] are mutually exclusive. (MESSAGE_ALLOWED_MENTIONS_PARSE_EXCLUSIVE)"); + + error_test.http_info.body = "{\ + \"message\": \"Invalid Form Body\",\ + \"code\": 50035,\ + \"errors\": {\ + \"1\": {\ + \"options\": {\ + \"1\": {\ + \"description\": {\ + \"_errors\": [\ + {\ + \"code\": \"BASE_TYPE_BAD_LENGTH\",\ + \"message\": \"Must be between 1 and 100 in length.\"\ + }\ + ]\ + }\ + }\ + }\ + }\ + }\ + }"; + error_message_success = (error_message_success && error_test.get_error().human_readable == "50035: Invalid Form Body - [1].options[1].description: Must be between 1 and 100 in length. (BASE_TYPE_BAD_LENGTH)"); + + set_test(ERRORS, error_message_success); + set_test(MD_ESC_1, false); set_test(MD_ESC_2, false); std::string escaped1 = dpp::utility::markdown_escape(test_to_escape); diff --git a/src/unittest/test.h b/src/unittest/test.h index c3b295ae45..df6a98d34a 100644 --- a/src/unittest/test.h +++ b/src/unittest/test.h @@ -144,6 +144,7 @@ DPP_TEST(CHANNELTYPES, "channel type flags", tf_online); DPP_TEST(FORUM_CREATION, "create a forum channel", tf_online); DPP_TEST(FORUM_CHANNEL_GET, "retrieve the created forum channel", tf_online); DPP_TEST(FORUM_CHANNEL_DELETE, "delete the created forum channel", tf_online); +DPP_TEST(ERRORS, "Human readable error translation", tf_offline); DPP_TEST(GUILD_BAN_CREATE, "cluster::guild_ban_add ban three deleted discord accounts", tf_online); DPP_TEST(GUILD_BAN_GET, "cluster::guild_get_ban getting one of the banned accounts", tf_online);