feat: GCS parse "error info" from JSON when available #7654
Conversation
Fixes: googleapis#7644 Please review this nlohmann code closely, because I've never used it and I may be doing things the least optimal way.
google-cla
bot
added
the
cla: yes
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7654 +/- ##
==========================================
- Coverage 95.28% 95.28% -0.01%
==========================================
Files 1254 1254
Lines 113227 113257 +30
==========================================
+ Hits 107893 107916 +23
- Misses 5334 5341 +7
Continue to review full report at Codecov.
|
| // ] | ||
| // See also https://cloud.google.com/apis/design/errors#http_mapping | ||
| ErrorInfo MakeErrorInfo(nlohmann::json const& details) { | ||
| static auto const* const kErrorInfoType = |
There was a problem hiding this comment.
nit: auto constexpr kErrorInfoType = "..."; seems like enough?
| if (v.value("@type", "") != kErrorInfoType) continue; | ||
| auto reason = v.value("reason", ""); | ||
| auto domain = v.value("domain", ""); | ||
| auto metadata_json = v.value("metadata", nlohmann::json::object()); |
There was a problem hiding this comment.
nit: I do not worry about copying here, this should be the uncommon path. But if one was worried about copying, there is a copy of something like a small std::map<>. You could do something like:
auto domain = v.value("domain", "");
if (!v.contains("metadata")) return ErrorInfo{std::move(reason), std::move(domain), {});
auto metadata = std::unordered_map<std::string, std::string>{};
for (auto const& m : v["metadata"].items()) {
metadata[m.key()] = m.value();
}
...There was a problem hiding this comment.
Thanks. Due to the likely minimal copying that would save, I think I'll leave the code as is, because I think it's possibly slightly easier to read as is.
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Fixes: #7644
Follows https://cloud.google.com/apis/design/errors#http_mapping and parses the "error info" json object from responses when it's available, and uses it to set the fields of the returned
g::c::Status.Note to reviewers: I'm new to using nlohmann::json, so please review that code closely and tell me what I'm doing wrong. I may be doing things very inefficiently or not following the right best practices. Please let me know.
This change is