diff options
| -rw-r--r-- | src/buildtool/build_engine/target_map/absent_target_map.cpp | 53 | ||||
| -rw-r--r-- | src/buildtool/build_engine/target_map/export.cpp | 61 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/target_client.cpp | 15 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/target_client.hpp | 11 |
4 files changed, 85 insertions, 55 deletions
diff --git a/src/buildtool/build_engine/target_map/absent_target_map.cpp b/src/buildtool/build_engine/target_map/absent_target_map.cpp index 1ab385c3..bd5c17a2 100644 --- a/src/buildtool/build_engine/target_map/absent_target_map.cpp +++ b/src/buildtool/build_engine/target_map/absent_target_map.cpp @@ -95,30 +95,39 @@ void WithFlexibleVariables( /*fatal=*/true); return; } - if (res->index() == 0) { - if (serve_failure_reporter != nullptr) { - (*serve_failure_reporter)(key, std::get<0>(*res)); + switch (auto const& ind = res->index(); ind) { + case 0: { + if (serve_failure_reporter != nullptr) { + (*serve_failure_reporter)(key, std::get<0>(*res)); + } + // target found but failed to analyse/build: log it as fatal + (*logger)( + fmt::format("Failure to remotely analyse or build absent " + "target {}\nDetailed log available on the " + "remote-execution endpoint as blob {}", + key.target.ToString(), + std::get<0>(*res)), + /*fatal=*/true); + return; + } + case 1: // fallthrough + case 2: { + // Other errors, including INTERNAL: log it as fatal + (*logger)(fmt::format( + "While querying serve endpoint for absent export " + "target {}:\n{}", + key.target.ToString(), + ind == 1 ? std::get<1>(*res) : std::get<2>(*res)), + /*fatal=*/true); + return; + } + default: { + // index == 3 + target_cache_value = std::get<3>(*res); + exports_progress->TaskTracker().Stop(task); + from_just_serve = true; } - (*logger)(fmt::format("Failure to remotely analyse or build absent " - "target {}\nDetailed log available on the " - "remote-execution endpoint as blob {}", - key.target.ToString(), - std::get<0>(*res)), - /*fatal=*/true); - return; - } - if (res->index() == 1) { - (*logger)(fmt::format("While querying serve endpoint for " - "absent export target {}:\n{}", - key.target.ToString(), - std::get<1>(*res)), - /*fatal=*/true); - return; } - // index == 2 - target_cache_value = std::get<2>(*res); - exports_progress->TaskTracker().Stop(task); - from_just_serve = true; } if (!target_cache_value) { diff --git a/src/buildtool/build_engine/target_map/export.cpp b/src/buildtool/build_engine/target_map/export.cpp index e0bc36c3..e92d4c9c 100644 --- a/src/buildtool/build_engine/target_map/export.cpp +++ b/src/buildtool/build_engine/target_map/export.cpp @@ -160,31 +160,44 @@ void ExportRule( key.target.ToString()); } else { - if (res->index() == 0) { - // target found but failed to analyse/build: this should be - // a fatal error for the local build too - (*logger)( - fmt::format("Failure to remotely analyse or build " - "target {}\nDetailed log available on the " - "remote-execution endpoint as blob {}", - key.target.ToString(), - std::get<0>(*res)), - /*fatal=*/true); - return; - } - if (res->index() == 1) { - // some other failure occurred while querying the serve - // endpoint; log to debug and continue locally - Logger::Log(LogLevel::Debug, - "While querying serve endpoint for export " - "target {}:\n{}", + switch (res->index()) { + case 0: { + // target found but failed to analyse/build: this should + // be a fatal error for the local build too + (*logger)( + fmt::format( + "Failure to remotely analyse or build target " + "{}\nDetailed log available on the " + "remote-execution endpoint as blob {}", key.target.ToString(), - std::get<1>(*res)); - } - else { - // index == 2 - target_cache_value = std::get<2>(*res); - from_just_serve = true; + std::get<0>(*res)), + /*fatal=*/true); + return; + } + case 1: { + // internal failure on the serve endpoint, or failures + // on the client side: local build should not continue + (*logger)(fmt::format("While querying serve endpoint " + "for export target {}:\n{}", + key.target.ToString(), + std::get<1>(*res)), + /*fatal=*/true); + return; + } + case 2: { + // some other failure occurred on the serve endpoint; + // log to debug and continue locally + Logger::Log(LogLevel::Debug, + "While querying serve endpoint for export " + "target {}:\n{}", + key.target.ToString(), + std::get<2>(*res)); + } break; + default: { + // index == 3 + target_cache_value = std::get<3>(*res); + from_just_serve = true; + } } } exports_progress->TaskTracker().Stop(task); diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp index d64f24c8..67b50a84 100644 --- a/src/buildtool/serve_api/remote/target_client.cpp +++ b/src/buildtool/serve_api/remote/target_client.cpp @@ -100,10 +100,6 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key, grpc::ClientContext context; justbuild::just_serve::ServeTargetResponse response; auto const& status = stub_->ServeTarget(&context, request, &response); - if (!status.ok()) { - return serve_target_result_t{std::in_place_index<1>, - StatusString(status)}; - } // differentiate status codes switch (status.error_code()) { @@ -145,7 +141,7 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key, auto const& result = TargetCacheEntry::FromJson( nlohmann::json::parse(*target_value_str)); // return the target cache value information - return serve_target_result_t{std::in_place_index<2>, + return serve_target_result_t{std::in_place_index<3>, std::make_pair(result, obj_info)}; } catch (std::exception const& ex) { return serve_target_result_t{ @@ -154,13 +150,20 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key, ex.what())}; } } + case grpc::StatusCode::INTERNAL: { + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format( + "Serve endpoint reported the fatal internal error:\n{}", + status.error_message())}; + } case grpc::StatusCode::NOT_FOUND: { return std::nullopt; // this might allow a local build to continue } default:; // fallthrough } return serve_target_result_t{ - std::in_place_index<1>, + std::in_place_index<2>, fmt::format("Serve endpoint failed with:\n{}", status.error_message())}; } diff --git a/src/buildtool/serve_api/remote/target_client.hpp b/src/buildtool/serve_api/remote/target_client.hpp index 948e9337..0d34ea2d 100644 --- a/src/buildtool/serve_api/remote/target_client.hpp +++ b/src/buildtool/serve_api/remote/target_client.hpp @@ -34,12 +34,17 @@ /// \brief Result union for the ServeTarget request. /// Index 0 will contain the hash of the blob containing the logged -/// analysis/build failure received from the endpoint, as a string. -/// Index 1 will contain any other failure message, as a string. -/// Index 2 will contain the target cache value information on success. +/// analysis/build failure received from the endpoint, as a string; this should +/// also trigger a local build fail. +/// Index 1 will contain the message of any INTERNAL error on the endpoint, as +/// a string; this should trigger a local build fail. +/// Index 2 will contain any other failure message, as a string; local builds +/// might be able to continue, but with a warning. +/// Index 3 will contain the target cache value information on success. using serve_target_result_t = std::variant<std::string, std::string, + std::string, std::pair<TargetCacheEntry, Artifact::ObjectInfo>>; /// Implements client side for Target service defined in: |