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 (#921)
  • Loading branch information
braindigitalis authored Oct 4, 2023
1 parent 4760452 commit 7bc1619
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 12 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"coroutine": "cpp",
"numbers": "cpp",
"semaphore": "cpp",
"stop_token": "cpp"
"stop_token": "cpp",
"charconv": "cpp"
}
}
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
74 changes: 65 additions & 9 deletions src/dpp/cluster/confirmation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,43 +82,99 @@ 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<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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;
detail.code = (*errordetails)["code"].get<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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<std::string>();
detail.reason = (*errordetails)["message"].get<std::string>();
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 + "- <array>[" + 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 + ")";
}
}

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
93 changes: 93 additions & 0 deletions src/unittest/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 - <array>[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);
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 7bc1619

Please sign in to comment.