Skip to content

Commit

Permalink
feat: improved human readable error messages to default logger, with …
Browse files Browse the repository at this point in the history
…unit test
  • Loading branch information
braindigitalis committed Oct 4, 2023
1 parent fa4fae3 commit 9062359
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 2 deletions.
8 changes: 8 additions & 0 deletions include/dpp/restresults.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ struct DPP_EXPORT error_detail {
* @brief Error reason (full message)
*/
std::string reason;
/**
* @brief Object field index
*/
int index = 0;
};

/**
Expand All @@ -235,6 +239,10 @@ struct DPP_EXPORT error_info {
* @brief Field specific error descriptions
*/
std::vector<error_detail> errors;
/**
* @brief Human readable error message constructed from the above
*/
std::string human_readable;
};

/**
Expand Down
17 changes: 17 additions & 0 deletions src/dpp/cluster/confirmation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ error_info confirmation_callback_t::get_error() const {
json& errors = j["errors"];
for (auto obj = errors.begin(); obj != errors.end(); ++obj) {

int array_index = 0;

Check notice on line 85 in src/dpp/cluster/confirmation.cpp

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/dpp/cluster/confirmation.cpp#L85

The scope of the variable 'array_index' can be reduced.

Check notice on line 85 in src/dpp/cluster/confirmation.cpp

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/dpp/cluster/confirmation.cpp#L85

Variable 'array_index' is assigned a value that is never used.
if (obj->find("0") != obj->end()) {
array_index = 0;
/* An array of error messages */
for (auto index = obj->begin(); index != obj->end(); ++index) {
if (index->find("_errors") != index->end()) {
Expand All @@ -92,6 +94,7 @@ error_info confirmation_callback_t::get_error() const {
detail.reason = (*errordetails)["message"].get<std::string>();
detail.object.clear();
detail.field = obj.key();
detail.index = array_index;
e.errors.emplace_back(detail);
}
} else {
Expand All @@ -102,10 +105,13 @@ error_info confirmation_callback_t::get_error() const {
detail.reason = (*errordetails)["message"].get<std::string>();
detail.field = fields.key();
detail.object = obj.key();
detail.index = array_index;
e.errors.emplace_back(detail);
}
}
}
/* Index only increments per field, not per error*/
array_index++;
}

} else if (obj->find("_errors") != obj->end()) {
Expand All @@ -117,11 +123,22 @@ error_info confirmation_callback_t::get_error() const {
detail.reason = (*errordetails)["message"].get<std::string>();
detail.object.clear();
detail.field = obj.key();
detail.index = 0;
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()) {
e.human_readable += prefix + "- " + error.field + ": " + error.reason + " (" + error.code + ")";
} else {
e.human_readable += prefix + "- " + error.object + "[" + std::to_string(error.index) + "]." + error.field + ": " + error.reason + " (" + error.code + ")";
}
}

return e;
}
return error_info();
Expand Down
4 changes: 2 additions & 2 deletions src/dpp/utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/unittest/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,25 @@ Markdown lol ||spoiler|| ~~strikethrough~~ `small *code* block`\n";
set_test(COMPARISON, u1 == u2 && u1 != u3);


dpp::confirmation_callback_t error_test;
set_test(ERRORS, false);
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)");

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);
Expand Down
1 change: 1 addition & 0 deletions src/unittest/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 9062359

Please sign in to comment.