Add error into Command::Abort method BUG:23906692 Change-Id: I45bd400343b3d6e0fa42c0f363f5d03381ada0c8 Reviewed-on: https://weave-review.googlesource.com/1257 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/libweave/examples/ubuntu/main.cc b/libweave/examples/ubuntu/main.cc index 28611f5..4c4d251 100644 --- a/libweave/examples/ubuntu/main.cc +++ b/libweave/examples/ubuntu/main.cc
@@ -141,8 +141,8 @@ if (!cmd) return; LOG(INFO) << "received command: " << cmd->GetName(); - int32_t led_index; - bool cmd_value; + int32_t led_index = 0; + bool cmd_value = false; if (cmd->GetParameters()->GetInteger("_led", &led_index) && cmd->GetParameters()->GetBoolean("_on", &cmd_value)) { // Display this command in terminal @@ -159,7 +159,10 @@ } return cmd->Done(); } - cmd->Abort(); + weave::ErrorPtr error; + weave::Error::AddTo(&error, FROM_HERE, "example", "invalid_parameter_value", + "Invalid parameters"); + cmd->Abort(error.get()); } void OnFlasherToggleCommand(const std::weak_ptr<weave::Command>& command) { @@ -167,7 +170,7 @@ if (!cmd) return; LOG(INFO) << "received command: " << cmd->GetName(); - int32_t led_index; + int32_t led_index = 0; if (cmd->GetParameters()->GetInteger("_led", &led_index)) { LOG(INFO) << cmd->GetName() << " _led: " << led_index; led_index--; @@ -176,7 +179,10 @@ UpdateLedState(); return cmd->Done(); } - cmd->Abort(); + weave::ErrorPtr error; + weave::Error::AddTo(&error, FROM_HERE, "example", "invalid_parameter_value", + "Invalid parameters"); + cmd->Abort(error.get()); } void OnUnhandledCommand(const std::weak_ptr<weave::Command>& command) {
diff --git a/libweave/include/weave/command.h b/libweave/include/weave/command.h index 6c399e1..eb4914f 100644 --- a/libweave/include/weave/command.h +++ b/libweave/include/weave/command.h
@@ -59,7 +59,7 @@ ErrorPtr* error) = 0; // Aborts command execution. - virtual void Abort() = 0; + virtual void Abort(const Error* error) = 0; // Cancels command execution. virtual void Cancel() = 0;
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc index dad443f..9e457e8 100644 --- a/libweave/src/base_api_handler.cc +++ b/libweave/src/base_api_handler.cc
@@ -7,6 +7,7 @@ #include <base/bind.h> #include <weave/device.h> +#include "src/commands/schema_constants.h" #include "src/device_registration_info.h" namespace weave { @@ -68,7 +69,12 @@ AuthScope auth_scope{AuthScope::kNone}; if (!StringToEnum(anonymous_access_role, &auth_scope)) { - return command->Abort(); + ErrorPtr error; + Error::AddToPrintf(&error, FROM_HERE, errors::commands::kDomain, + errors::commands::kInvalidPropValue, + "Invalid localAnonymousAccessMaxRole value '%s'", + anonymous_access_role.c_str()); + return command->Abort(error.get()); } device_info_->UpdateBaseConfig(auth_scope, discovery_enabled,
diff --git a/libweave/src/commands/command_instance.cc b/libweave/src/commands/command_instance.cc index 305a255..45c376d 100644 --- a/libweave/src/commands/command_instance.cc +++ b/libweave/src/commands/command_instance.cc
@@ -16,6 +16,7 @@ #include "src/commands/schema_constants.h" #include "src/commands/schema_utils.h" #include "src/json_error_codes.h" +#include "src/utils.h" namespace weave { @@ -250,6 +251,10 @@ json->Set(commands::attributes::kCommand_Results, TypedValueToJson(results_).release()); json->SetString(commands::attributes::kCommand_State, EnumToString(status_)); + if (error_) { + json->Set(commands::attributes::kCommand_Error, + ErrorInfoToJson(*error_).release()); + } return json; } @@ -262,7 +267,9 @@ observers_.RemoveObserver(observer); } -void CommandInstance::Abort() { +void CommandInstance::Abort(const Error* error) { + if (error) + error_ = error->Clone(); SetStatus(CommandStatus::kAborted); RemoveFromQueue(); // The command will be destroyed after that, so do not access any members.
diff --git a/libweave/src/commands/command_instance.h b/libweave/src/commands/command_instance.h index 20e1daa..c2a79a8 100644 --- a/libweave/src/commands/command_instance.h +++ b/libweave/src/commands/command_instance.h
@@ -64,7 +64,7 @@ ErrorPtr* error) override; bool SetResults(const base::DictionaryValue& results, ErrorPtr* error) override; - void Abort() override; + void Abort(const Error* error) override; void Cancel() override; void Done() override; @@ -128,6 +128,8 @@ ValueMap results_; // Current command status. CommandStatus status_ = CommandStatus::kQueued; + // Error encountered during execution of the command. + ErrorPtr error_; // Command observers. base::ObserverList<Observer> observers_; // Pointer to the command queue this command instance is added to.
diff --git a/libweave/src/commands/command_instance_unittest.cc b/libweave/src/commands/command_instance_unittest.cc index 865ba6e..f783ca7 100644 --- a/libweave/src/commands/command_instance_unittest.cc +++ b/libweave/src/commands/command_instance_unittest.cc
@@ -212,11 +212,16 @@ EXPECT_TRUE(instance->SetResults(*CreateDictionaryValue("{'testResult': 17}"), nullptr)); + ErrorPtr error; + Error::AddTo(&error, FROM_HERE, "DOMAIN", "CODE", "MESSAGE"); + instance->Abort(error.get()); + json->MergeDictionary(CreateDictionaryValue(R"({ 'id': 'testId', 'progress': {'progress': 15}, - 'state': 'inProgress', - 'results': {'testResult': 17} + 'state': 'aborted', + 'results': {'testResult': 17}, + 'error': {'code': 'CODE', 'message': 'MESSAGE'} })").get()); auto converted = instance->ToJson();
diff --git a/libweave/src/commands/schema_constants.cc b/libweave/src/commands/schema_constants.cc index 02999f7..f53423f 100644 --- a/libweave/src/commands/schema_constants.cc +++ b/libweave/src/commands/schema_constants.cc
@@ -57,6 +57,7 @@ const char kCommand_Progress[] = "progress"; const char kCommand_Results[] = "results"; const char kCommand_State[] = "state"; +const char kCommand_Error[] = "error"; const char kCommand_Role[] = "minimalRole"; const char kCommand_Role_Manager[] = "manager"; @@ -64,9 +65,6 @@ const char kCommand_Role_User[] = "user"; const char kCommand_Role_Viewer[] = "viewer"; -const char kCommand_ErrorCode[] = "error.code"; -const char kCommand_ErrorMessage[] = "error.message"; - const char kCommand_Visibility[] = "visibility"; const char kCommand_Visibility_None[] = "none"; const char kCommand_Visibility_Local[] = "local";
diff --git a/libweave/src/commands/schema_constants.h b/libweave/src/commands/schema_constants.h index 9eb8051..6dded05 100644 --- a/libweave/src/commands/schema_constants.h +++ b/libweave/src/commands/schema_constants.h
@@ -60,6 +60,7 @@ extern const char kCommand_Progress[]; extern const char kCommand_Results[]; extern const char kCommand_State[]; +extern const char kCommand_Error[]; extern const char kCommand_Role[]; extern const char kCommand_Role_Manager[]; @@ -67,9 +68,6 @@ extern const char kCommand_Role_User[]; extern const char kCommand_Role_Viewer[]; -extern const char kCommand_ErrorCode[]; -extern const char kCommand_ErrorMessage[]; - extern const char kCommand_Visibility[]; extern const char kCommand_Visibility_None[]; extern const char kCommand_Visibility_Local[];
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc index e9e267a..c83b7b6 100644 --- a/libweave/src/device_registration_info.cc +++ b/libweave/src/device_registration_info.cc
@@ -862,16 +862,8 @@ command_patch.SetString(commands::attributes::kCommand_State, EnumToString(CommandStatus::kAborted)); if (error) { - command_patch.SetString(commands::attributes::kCommand_ErrorCode, - Join(":", error->GetDomain(), error->GetCode())); - std::vector<std::string> messages; - const Error* current_error = error.get(); - while (current_error) { - messages.push_back(current_error->GetMessage()); - current_error = current_error->GetInnerError(); - } - command_patch.SetString(commands::attributes::kCommand_ErrorMessage, - Join(";", messages)); + command_patch.Set(commands::attributes::kCommand_Error, + ErrorInfoToJson(*error).release()); } UpdateCommand(command_id, command_patch, base::Bind(&base::DoNothing), base::Bind(&base::DoNothing));
diff --git a/libweave/src/privet/privet_handler.cc b/libweave/src/privet/privet_handler.cc index 9475cfe..a7bfee1 100644 --- a/libweave/src/privet/privet_handler.cc +++ b/libweave/src/privet/privet_handler.cc
@@ -23,6 +23,7 @@ #include "src/privet/security_delegate.h" #include "src/privet/wifi_delegate.h" #include "src/string_utils.h" +#include "src/utils.h" namespace weave { namespace privet { @@ -90,8 +91,6 @@ const char kAuthorizationHeaderPrefix[] = "Privet"; -const char kErrorCodeKey[] = "code"; -const char kErrorMessageKey[] = "message"; const char kErrorDebugInfoKey[] = "debugInfo"; const char kSetupStartSsidKey[] = "ssid"; @@ -156,13 +155,6 @@ return SplitAtFirst(auth_header, " ", true).second; } -std::unique_ptr<base::DictionaryValue> ErrorInfoToJson(const Error& error) { - std::unique_ptr<base::DictionaryValue> output{new base::DictionaryValue}; - output->SetString(kErrorMessageKey, error.GetMessage()); - output->SetString(kErrorCodeKey, error.GetCode()); - return output; -} - // Creates JSON similar to GCD server error format. std::unique_ptr<base::DictionaryValue> ErrorToJson(const Error& error) { std::unique_ptr<base::DictionaryValue> output{ErrorInfoToJson(error)};
diff --git a/libweave/src/utils.cc b/libweave/src/utils.cc index 6827db5..20debea 100644 --- a/libweave/src/utils.cc +++ b/libweave/src/utils.cc
@@ -23,6 +23,9 @@ const size_t kMaxStrLen = 1700; // Log messages are limited to 2000 chars. +const char kErrorCodeKey[] = "code"; +const char kErrorMessageKey[] = "message"; + } // anonymous namespace namespace errors { @@ -62,4 +65,11 @@ return result; } +std::unique_ptr<base::DictionaryValue> ErrorInfoToJson(const Error& error) { + std::unique_ptr<base::DictionaryValue> output{new base::DictionaryValue}; + output->SetString(kErrorMessageKey, error.GetMessage()); + output->SetString(kErrorCodeKey, error.GetCode()); + return output; +} + } // namespace weave
diff --git a/libweave/src/utils.h b/libweave/src/utils.h index 29ea138..11f9483 100644 --- a/libweave/src/utils.h +++ b/libweave/src/utils.h
@@ -30,6 +30,8 @@ const std::string& json_string, ErrorPtr* error); +std::unique_ptr<base::DictionaryValue> ErrorInfoToJson(const Error& error); + } // namespace weave #endif // LIBWEAVE_SRC_UTILS_H_