Skip to content

Commit

Permalink
Change the error handling location from DataLoader to Engine.
Browse files Browse the repository at this point in the history
* Engine::MaybeReloadEngine takes care of all error handing of DataLoader::Response.
* This is a preparation CL for the next CL (cl/615733907).

#codehealth

PiperOrigin-RevId: 617099369
  • Loading branch information
hiroyuki-komatsu committed Mar 19, 2024
1 parent 09df0ab commit cbd7e33
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/engine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ mozc_cc_library(
"//base:file_util",
"//base:hash",
"//base:thread",
"//base/protobuf:message",
"//data_manager",
"//protocol:engine_builder_cc_proto",
"@com_google_absl//absl/base:core_headers",
Expand Down Expand Up @@ -172,6 +171,7 @@ mozc_cc_library(
":spellchecker_interface",
":user_data_manager_interface",
"//base:vlog",
"//base/protobuf:message",
"//converter",
"//converter:converter_interface",
"//converter:immutable_converter_interface",
Expand Down
11 changes: 0 additions & 11 deletions src/engine/data_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "absl/synchronization/mutex.h"
#include "base/file_util.h"
#include "base/hash.h"
#include "base/protobuf/message.h"
#include "base/thread.h"
#include "data_manager/data_manager.h"
#include "engine/modules.h"
Expand Down Expand Up @@ -227,16 +226,6 @@ DataLoader::MaybeMoveDataLoaderResponse() {
std::move(*loader_response_future_).Get();
loader_response_future_.reset();

if (!loader_response.modules ||
loader_response.response.status() != EngineReloadResponse::RELOAD_READY) {
// The loader_response does not contain a valid result.

// This engine id causes a critical error, so rollback the id.
LOG(ERROR) << "Failure in engine loading: "
<< protobuf::Utf8Format(loader_response.response);
ReportLoadFailure(loader_response.id);
return nullptr;
}
return std::make_unique<DataLoader::Response>(std::move(loader_response));
}

Expand Down
16 changes: 14 additions & 2 deletions src/engine/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "base/protobuf/message.h"
#include "base/vlog.h"
#include "converter/converter.h"
#include "converter/immutable_converter.h"
Expand Down Expand Up @@ -281,13 +282,24 @@ bool Engine::MaybeReloadEngine(EngineReloadResponse *response) {
}
*response = loader_response->response;

if (!loader_response->modules ||
response->status() != EngineReloadResponse::RELOAD_READY) {
// The loader_response does not contain a valid result.

// This request id causes a critical error, so rollback the id.
LOG(ERROR) << "Failure in loading response: "
<< protobuf::Utf8Format(*response);
loader_->ReportLoadFailure(loader_response->id);
return false;
}

if (user_data_manager_) {
user_data_manager_->Wait();
}

// Reloads DataManager.
const bool is_mobile = loader_response->response.request().engine_type() ==
EngineReloadRequest::MOBILE;
const bool is_mobile =
response->request().engine_type() == EngineReloadRequest::MOBILE;
absl::Status reload_status =
ReloadModules(std::move(loader_response->modules), is_mobile);
if (!reload_status.ok()) {
Expand Down

0 comments on commit cbd7e33

Please sign in to comment.