Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improved human readable error messages to default logger, with unit test #921

Merged
merged 3 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading