Commit 83f295d5 authored by Jae Hoon Kim's avatar Jae Hoon Kim Committed by Commit Bot

Fix Error Propagation from dlcservice daemon

Once the dependent CQ is uprev'ed, the DBus errors will be correctly
handled and logged in a clearer fashion. The parsing logic is not
required since the errors are individual accessible within the
ErrorResponse itself.

Bug: none
Test: # with CrOS changes + manual testing
Change-Id: Id95362c8a2acbb63bc45f11e60313724e558d461
Cq-Depend: chromium:1848202, chromium:1865202
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848577
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706569}
parent 715c6640
...@@ -44,25 +44,25 @@ DlcserviceClient* g_instance = nullptr; ...@@ -44,25 +44,25 @@ DlcserviceClient* g_instance = nullptr;
class DlcserviceErrorResponseHandler { class DlcserviceErrorResponseHandler {
public: public:
explicit DlcserviceErrorResponseHandler(dbus::ErrorResponse* err_response) explicit DlcserviceErrorResponseHandler(dbus::ErrorResponse* err_response)
: err(dlcservice::kErrorInternal) { : err_(dlcservice::kErrorInternal) {
if (err_response && dbus::MessageReader(err_response).PopString(&err_msg) && if (!err_response) {
DlcserviceErrorFromString(err_msg, &err)) { LOG(ERROR) << "Failed to set err since ErrorResponse is null.";
VLOG(1) << "Handling error response err=" << err return;
<< " err_msg=" << err_msg;
} else {
LOG(ERROR) << "Failed to set err based on error response "
<< "defaulted to kErrorInternal.";
} }
VerifyAndSetError(err_response);
VerifyAndSetErrorMessage(err_response);
VLOG(1) << "Handling err=" << err_ << " err_msg=" << err_msg_;
} }
~DlcserviceErrorResponseHandler() = default; ~DlcserviceErrorResponseHandler() = default;
std::string get_err() { return err; } std::string get_err() { return err_; }
std::string get_err_msg() { return err_msg; } std::string get_err_msg() { return err_msg_; }
private: private:
bool DlcserviceErrorFromString(const std::string& err_msg, std::string* err) { void VerifyAndSetError(dbus::ErrorResponse* err_response) {
const std::string& err = err_response->GetErrorName();
static const base::NoDestructor<std::unordered_set<std::string>> err_set({ static const base::NoDestructor<std::unordered_set<std::string>> err_set({
dlcservice::kErrorNone, dlcservice::kErrorNone,
dlcservice::kErrorInternal, dlcservice::kErrorInternal,
...@@ -70,40 +70,28 @@ class DlcserviceErrorResponseHandler { ...@@ -70,40 +70,28 @@ class DlcserviceErrorResponseHandler {
dlcservice::kErrorNeedReboot, dlcservice::kErrorNeedReboot,
dlcservice::kErrorInvalidDlc, dlcservice::kErrorInvalidDlc,
}); });
static const std::pair<std::string, std::string> delims = {"dlcservice/", // Lookup the dlcservice error code and provide default on invalid.
":"}; auto itr = err_set->find(err);
if (itr == err_set->end()) {
if (!err) { LOG(ERROR) << "Failed to set error based on ErrorResponse "
LOG(ERROR) << "err passed is nullptr."; "defaulted to kErrorInternal, was:" << err;
return false; err_ = dlcservice::kErrorInternal;
return;
} }
err_ = *itr;
// Clear to empty out |err|.
err->clear();
// Verify dlcservice error code.
if (err_msg.find(delims.first) == std::string::npos) {
LOG(ERROR) << "Dlcservice did not send valid error message: " << err_msg;
*err = dlcservice::kErrorInternal;
return true;
} }
// Extract the dlcservice error code. void VerifyAndSetErrorMessage(dbus::ErrorResponse* err_response) {
size_t padding = delims.first.size(); if (!dbus::MessageReader(err_response).PopString(&err_msg_)) {
size_t second_idx = err_msg.find(delims.second, padding); LOG(ERROR) << "Failed to set error message from ErrorResponse.";
*err = err_msg.substr(padding, second_idx - padding); }
// Lookup the dlcservice error code and provide default on failure.
auto itr = err_set->find(*err);
*err = itr != err_set->end() ? *itr : dlcservice::kErrorInternal;
return true;
} }
// Holds the dlcservice specific error. // Holds the dlcservice specific error.
std::string err; std::string err_;
// Holds the entire error message from error response. // Holds the entire error message from error response.
std::string err_msg; std::string err_msg_;
DISALLOW_COPY_AND_ASSIGN(DlcserviceErrorResponseHandler); DISALLOW_COPY_AND_ASSIGN(DlcserviceErrorResponseHandler);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment