Simplify Expand Handling and Propogate Error Message Correctly
Before our error messages were not following the Redfish spec, so we did not
propagate the errors correctly. Now we follow the redfish spec and follow upstream
gbmcweb's implementation here:
https://gbmc.googlesource.com/gbmcweb/+/refs/heads/master/redfish-core/src/error_messages.cpp#89
Some refactoring is done for easier reading of the code.
#tlbmc
Errors correctly propagated in real machine:
https://paste.googleplex.com/5489501925015552
PiperOrigin-RevId: 775463387
Change-Id: I37d22d33b75f8caf2b1673ee74285b527acfd9f4
diff --git a/tlbmc/redfish/expand_query_aggregator.cc b/tlbmc/redfish/expand_query_aggregator.cc
index c1b564f..f5245f1 100644
--- a/tlbmc/redfish/expand_query_aggregator.cc
+++ b/tlbmc/redfish/expand_query_aggregator.cc
@@ -62,32 +62,84 @@
void ExpandQueryAggregator::AddResponseToFinalResponse(
const nlohmann::json::json_pointer& location, Response& sub_response) {
- PropogateErrorCodeIfNeeded(location, sub_response);
- if (!sub_response.jsonValue.is_object() || sub_response.jsonValue.empty()) {
- return;
+ // Special handling for subresponse error
+ if (sub_response.resultInt() < 200 || sub_response.resultInt() >= 400) {
+ DLOG(INFO) << "Sub response error: " << sub_response.resultInt() << " "
+ << sub_response.jsonValue.dump();
+ // Determine the final error code
+ final_async_resp_->res.result(
+ DetermineFinalErrorCode(sub_response.resultInt()));
+
+ // Move Error Messages to final response
+ PropagateErrorMessage(sub_response.jsonValue);
+
+ // Remove error from subresponse before moving
+ auto error = sub_response.jsonValue.find("error");
+ if (error != sub_response.jsonValue.end()) {
+ sub_response.jsonValue.erase(error);
+ }
}
final_async_resp_->res.jsonValue[location] =
std::move(sub_response.jsonValue);
}
-// Make error codes distinct between the query made.
-void ExpandQueryAggregator::PropogateErrorCodeIfNeeded(
- const nlohmann::json::json_pointer& location, Response& sub_response) {
- if (sub_response.resultInt() >= 200 && sub_response.resultInt() < 400) {
- return;
+ErrorInformation ExpandQueryAggregator::GetErrorInformation(
+ const nlohmann::json& response_json) {
+ ErrorInformation error_information;
+ auto error = response_json.find("error");
+ if (error != response_json.end()) {
+ auto message = error->find("message");
+ if (message != error->end()) {
+ error_information.error_message = *message;
+ }
+
+ auto code = error->find("code");
+ if (code != error->end()) {
+ error_information.error_code = *code;
+ }
+
+ auto extended_info = error->find("@Message.ExtendedInfo");
+ if (extended_info != error->end() && extended_info->is_array()) {
+ error_information.extended_info = *extended_info;
+ }
+ }
+ return error_information;
+}
+
+void ExpandQueryAggregator::PropagateErrorMessage(
+ nlohmann::json& sub_response_json) {
+ nlohmann::json& final_response_json_error =
+ final_async_resp_->res.jsonValue["error"];
+
+ ErrorInformation error_information = GetErrorInformation(sub_response_json);
+
+ // If the final response doesn't have an error object, create and populate
+ if (!final_response_json_error.is_object()) {
+ final_response_json_error["code"] = error_information.error_code;
+ final_response_json_error["message"] = error_information.error_message;
+ } else {
+ // Else use generic error code and message
+ final_response_json_error["code"] = "Base.1.11.0.GeneralError";
+ final_response_json_error["message"] =
+ "A general error has occurred. See Resolution for information on how "
+ "to resolve the error.";
}
- final_async_resp_->res.result(
- DetermineFinalErrorCode(sub_response.resultInt()));
+ // Populate the Extended Info
+ nlohmann::json& final_extended_info =
+ final_response_json_error["@Message.ExtendedInfo"];
- if (sub_response.jsonValue.contains("error")) {
- if (!final_async_resp_->res.jsonValue.contains("error")) {
- final_async_resp_->res.jsonValue["error"] = nlohmann::json::object();
- }
- final_async_resp_->res.jsonValue["error"][location] =
- std::move(sub_response.jsonValue["error"]);
- sub_response.jsonValue.clear();
+ // Create Extended Info array if it doesn't exist
+ if (!final_extended_info.is_array()) {
+ final_extended_info = nlohmann::json::array();
+ }
+
+ // Populate subresponse's extended info into the final response's extended
+ // info
+ for (const auto& sub_response_extended_info :
+ error_information.extended_info) {
+ final_extended_info.push_back(sub_response_extended_info);
}
}
diff --git a/tlbmc/redfish/expand_query_aggregator.h b/tlbmc/redfish/expand_query_aggregator.h
index 386b4f6..3f70909 100644
--- a/tlbmc/redfish/expand_query_aggregator.h
+++ b/tlbmc/redfish/expand_query_aggregator.h
@@ -5,6 +5,7 @@
#include <string>
#include <utility>
+#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "nlohmann/json_fwd.hpp"
#include "tlbmc/redfish/query_parameters.h"
@@ -15,6 +16,17 @@
namespace milotic_tlbmc {
+constexpr absl::string_view kDefaultErrorMessage =
+ "A general error has occurred. See Resolution for information on how to "
+ "resolve the error.";
+constexpr absl::string_view kDefaultErrorCode = "Base.1.11.0.GeneralError";
+
+struct ErrorInformation {
+ std::string error_message = std::string(kDefaultErrorMessage);
+ std::string error_code = std::string(kDefaultErrorCode);
+ nlohmann::json::array_t extended_info;
+};
+
// This class will be in charge of aggregating the responses from the expand
// query.
// This class references the implmentation in openbmc gbmcweb.
@@ -34,15 +46,17 @@
void StartQuery();
protected:
+ static ErrorInformation GetErrorInformation(
+ const nlohmann::json& response_json);
+
+ void PropagateErrorMessage(nlohmann::json& sub_response_json);
+
std::string CreateExpandQueryUrl(absl::string_view url,
const ExpandNode& expand_node);
void AddResponseToFinalResponse(const nlohmann::json::json_pointer& location,
::crow::Response& sub_response);
- void PropogateErrorCodeIfNeeded(const nlohmann::json::json_pointer& location,
- ::crow::Response& sub_response);
-
unsigned DetermineFinalErrorCode(unsigned sub_response_code);
std::shared_ptr<bmcweb::AsyncResp> final_async_resp_;
