From 2344f8e5289ccd64f2eb3594aac3fee62f76395a Mon Sep 17 00:00:00 2001 From: Kane York Date: Mon, 3 Aug 2020 08:18:56 -0700 Subject: feat(3p/nix/daemon): catch-all explicit Error-Status conversion We wrap every server-side proto handler with a macro that catches exceptions and turns them into proper grpc error codes. For the time being, most exceptions map to INTERNAL, the existing mapping. Change-Id: Id6ed6a279b198ad185d32562f39000ccc15eadbf Reviewed-on: https://cl.tvl.fyi/c/depot/+/1599 Tested-by: BuildkiteCI Reviewed-by: glittershark --- third_party/nix/src/nix-daemon/nix-daemon-proto.cc | 548 ++++++++++++--------- 1 file changed, 318 insertions(+), 230 deletions(-) (limited to 'third_party/nix/src/nix-daemon') diff --git a/third_party/nix/src/nix-daemon/nix-daemon-proto.cc b/third_party/nix/src/nix-daemon/nix-daemon-proto.cc index 01a022a30893..2a21c7f91c6f 100644 --- a/third_party/nix/src/nix-daemon/nix-daemon-proto.cc +++ b/third_party/nix/src/nix-daemon/nix-daemon-proto.cc @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -29,9 +30,6 @@ using ::nix::proto::StorePath; using ::nix::proto::StorePaths; using ::nix::proto::WorkerService; -static Status INVALID_STORE_PATH = - Status(grpc::StatusCode::INVALID_ARGUMENT, "Invalid store path"); - class AddToStoreRequestSource final : public Source { using Reader = grpc::ServerReader; @@ -96,86 +94,103 @@ class WorkerServiceImpl final : public WorkerService::Service { Status IsValidPath(grpc::ServerContext* context, const StorePath* request, nix::proto::IsValidPathResponse* response) override { - const auto& path = request->path(); - response->set_is_valid(store_->isValidPath(path)); - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + const auto& path = request->path(); + response->set_is_valid(store_->isValidPath(path)); + + return Status::OK; + }, + __FUNCTION__); } Status HasSubstitutes(grpc::ServerContext* context, const StorePath* request, nix::proto::HasSubstitutesResponse* response) override { - const auto& path = request->path(); - ASSERT_INPUT_STORE_PATH(path); - PathSet res = store_->querySubstitutablePaths({path}); - response->set_has_substitutes(res.find(path) != res.end()); - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + const auto& path = request->path(); + ASSERT_INPUT_STORE_PATH(path); + PathSet res = store_->querySubstitutablePaths({path}); + response->set_has_substitutes(res.find(path) != res.end()); + + return Status::OK; + }, + __FUNCTION__); } Status QueryReferrers(grpc::ServerContext* context, const StorePath* request, StorePaths* response) override { - const auto& path = request->path(); - ASSERT_INPUT_STORE_PATH(path); + return HandleExceptions( + [&]() -> Status { + const auto& path = request->path(); + ASSERT_INPUT_STORE_PATH(path); - PathSet paths; - store_->queryReferrers(path, paths); + PathSet paths; + store_->queryReferrers(path, paths); - for (const auto& path : paths) { - response->add_paths(path); - } + for (const auto& path : paths) { + response->add_paths(path); + } - return Status::OK; + return Status::OK; + }, + __FUNCTION__); } Status AddToStore(grpc::ServerContext* context, grpc::ServerReader* reader, nix::proto::StorePath* response) override { - proto::AddToStoreRequest metadata_request; - auto has_metadata = reader->Read(&metadata_request); - - if (!has_metadata || metadata_request.has_meta()) { - return Status(grpc::StatusCode::INVALID_ARGUMENT, - "Metadata must be set before sending file content"); - } - - auto meta = metadata_request.meta(); - AddToStoreRequestSource source(reader); - auto opt_hash_type = hash_type_from(meta.hash_type()); - if (!opt_hash_type) { - return Status(grpc::StatusCode::INTERNAL, "Invalid hash type"); - } - - std::string* data; - RetrieveRegularNARSink nar; - TeeSource saved_nar(source); - - if (meta.recursive()) { - // TODO(grfn): Don't store the full data in memory, instead just make - // addToStoreFromDump take a Source - ParseSink sink; - parseDump(sink, saved_nar); - data = &(*saved_nar.data); - } else { - parseDump(nar, source); - if (!nar.regular) { - return Status(grpc::StatusCode::INVALID_ARGUMENT, - "Regular file expected"); - } - data = &nar.s; - } - - auto local_store = store_.dynamic_pointer_cast(); - if (!local_store) { - return Status(grpc::StatusCode::FAILED_PRECONDITION, - "operation is only supported by LocalStore"); - } - - auto path = local_store->addToStoreFromDump( - *data, meta.base_name(), meta.recursive(), opt_hash_type.value()); - - response->set_path(path); - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + proto::AddToStoreRequest metadata_request; + auto has_metadata = reader->Read(&metadata_request); + + if (!has_metadata || metadata_request.has_meta()) { + return Status(grpc::StatusCode::INVALID_ARGUMENT, + "Metadata must be set before sending file content"); + } + + auto meta = metadata_request.meta(); + AddToStoreRequestSource source(reader); + auto opt_hash_type = hash_type_from(meta.hash_type()); + if (!opt_hash_type) { + return Status(grpc::StatusCode::INVALID_ARGUMENT, + "Invalid hash type"); + } + + std::string* data; + RetrieveRegularNARSink nar; + TeeSource saved_nar(source); + + if (meta.recursive()) { + // TODO(grfn): Don't store the full data in memory, instead just + // make addToStoreFromDump take a Source + ParseSink sink; + parseDump(sink, saved_nar); + data = &(*saved_nar.data); + } else { + parseDump(nar, source); + if (!nar.regular) { + return Status(grpc::StatusCode::INVALID_ARGUMENT, + "Regular file expected"); + } + data = &nar.s; + } + + auto local_store = store_.dynamic_pointer_cast(); + if (!local_store) { + return Status(grpc::StatusCode::FAILED_PRECONDITION, + "operation is only supported by LocalStore"); + } + + auto path = local_store->addToStoreFromDump( + *data, meta.base_name(), meta.recursive(), opt_hash_type.value()); + + response->set_path(path); + + return Status::OK; + }, + __FUNCTION__); } Status AddTextToStore(grpc::ServerContext*, @@ -215,247 +230,320 @@ class WorkerServiceImpl final : public WorkerService::Service { Status QuerySubstitutablePathInfos( grpc::ServerContext*, const StorePaths* request, nix::proto::SubstitutablePathInfos* response) override { - SubstitutablePathInfos infos; - PathSet paths; - for (const auto& path : request->paths()) { - paths.insert(path); - } - store_->querySubstitutablePathInfos(paths, infos); - for (const auto& [path, path_info] : infos) { - auto proto_path_info = response->add_path_infos(); - proto_path_info->mutable_path()->set_path(path); - proto_path_info->mutable_deriver()->set_path(path_info.deriver); - for (const auto& ref : path_info.references) { - proto_path_info->add_references(ref); - } - proto_path_info->set_download_size(path_info.downloadSize); - proto_path_info->set_nar_size(path_info.narSize); - } - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + SubstitutablePathInfos infos; + PathSet paths; + for (const auto& path : request->paths()) { + paths.insert(path); + } + store_->querySubstitutablePathInfos(paths, infos); + for (const auto& [path, path_info] : infos) { + auto proto_path_info = response->add_path_infos(); + proto_path_info->mutable_path()->set_path(path); + proto_path_info->mutable_deriver()->set_path(path_info.deriver); + for (const auto& ref : path_info.references) { + proto_path_info->add_references(ref); + } + proto_path_info->set_download_size(path_info.downloadSize); + proto_path_info->set_nar_size(path_info.narSize); + } + + return Status::OK; + }, + __FUNCTION__); } Status QueryValidDerivers(grpc::ServerContext* context, const StorePath* request, StorePaths* response) override { - const auto& path = request->path(); - ASSERT_INPUT_STORE_PATH(path); + return HandleExceptions( + [&]() -> Status { + const auto& path = request->path(); + ASSERT_INPUT_STORE_PATH(path); - PathSet paths = store_->queryValidDerivers(path); + PathSet paths = store_->queryValidDerivers(path); - for (const auto& path : paths) { - response->add_paths(path); - } + for (const auto& path : paths) { + response->add_paths(path); + } - return Status::OK; + return Status::OK; + }, + __FUNCTION__); } Status QueryDerivationOutputs(grpc::ServerContext* context, const StorePath* request, StorePaths* response) override { - const auto& path = request->path(); - ASSERT_INPUT_STORE_PATH(path); + return HandleExceptions( + [&]() -> Status { + const auto& path = request->path(); + ASSERT_INPUT_STORE_PATH(path); - PathSet paths = store_->queryDerivationOutputs(path); + PathSet paths = store_->queryDerivationOutputs(path); - for (const auto& path : paths) { - response->add_paths(path); - } + for (const auto& path : paths) { + response->add_paths(path); + } - return Status::OK; + return Status::OK; + }, + __FUNCTION__); } Status QueryAllValidPaths(grpc::ServerContext* context, const google::protobuf::Empty* request, StorePaths* response) override { - const auto paths = store_->queryAllValidPaths(); - for (const auto& path : paths) { - response->add_paths(path); - } - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + const auto paths = store_->queryAllValidPaths(); + for (const auto& path : paths) { + response->add_paths(path); + } + + return Status::OK; + }, + __FUNCTION__); } Status QueryPathInfo(grpc::ServerContext* context, const StorePath* request, PathInfo* response) override { - auto path = request->path(); - ASSERT_INPUT_STORE_PATH(path); - - response->mutable_path()->set_path(path); - try { - auto info = store_->queryPathInfo(path); - response->mutable_deriver()->set_path(info->deriver); - response->set_nar_hash( - reinterpret_cast(&info->narHash.hash[0]), - info->narHash.hashSize); - - for (const auto& reference : info->references) { - response->add_references(reference); - } - - *response->mutable_registration_time() = - google::protobuf::util::TimeUtil::TimeTToTimestamp( - info->registrationTime); - - response->set_nar_size(info->narSize); - response->set_ultimate(info->ultimate); - - for (const auto& sig : info->sigs) { - response->add_sigs(sig); - } - - response->set_ca(info->ca); - - return Status::OK; - } catch (InvalidPath&) { - return Status(grpc::StatusCode::INVALID_ARGUMENT, "Invalid store path"); - } + return HandleExceptions( + [&]() -> Status { + auto path = request->path(); + ASSERT_INPUT_STORE_PATH(path); + + response->mutable_path()->set_path(path); + try { + auto info = store_->queryPathInfo(path); + response->mutable_deriver()->set_path(info->deriver); + response->set_nar_hash( + reinterpret_cast(&info->narHash.hash[0]), + info->narHash.hashSize); + + for (const auto& reference : info->references) { + response->add_references(reference); + } + + *response->mutable_registration_time() = + google::protobuf::util::TimeUtil::TimeTToTimestamp( + info->registrationTime); + + response->set_nar_size(info->narSize); + response->set_ultimate(info->ultimate); + + for (const auto& sig : info->sigs) { + response->add_sigs(sig); + } + + response->set_ca(info->ca); + + return Status::OK; + } catch (InvalidPath&) { + return Status(grpc::StatusCode::INVALID_ARGUMENT, + "Invalid store path"); + } + }, + __FUNCTION__); } Status QueryDerivationOutputNames( grpc::ServerContext* context, const StorePath* request, nix::proto::DerivationOutputNames* response) override { - auto path = request->path(); - ASSERT_INPUT_STORE_PATH(path); - auto names = store_->queryDerivationOutputNames(path); - for (const auto& name : names) { - response->add_names(name); - } - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + auto path = request->path(); + ASSERT_INPUT_STORE_PATH(path); + auto names = store_->queryDerivationOutputNames(path); + for (const auto& name : names) { + response->add_names(name); + } + + return Status::OK; + }, + __FUNCTION__); } Status QueryPathFromHashPart(grpc::ServerContext* context, const nix::proto::HashPart* request, StorePath* response) override { - auto hash_part = request->hash_part(); - auto path = store_->queryPathFromHashPart(hash_part); - ASSERT_INPUT_STORE_PATH(path); - response->set_path(path); - return Status::OK; + return HandleExceptions( + [&]() -> Status { + auto hash_part = request->hash_part(); + auto path = store_->queryPathFromHashPart(hash_part); + ASSERT_INPUT_STORE_PATH(path); + response->set_path(path); + return Status::OK; + }, + __FUNCTION__); } Status QueryValidPaths(grpc::ServerContext* context, const StorePaths* request, StorePaths* response) override { - std::set paths; - for (const auto& path : request->paths()) { - ASSERT_INPUT_STORE_PATH(path); - paths.insert(path); - } - - auto res = store_->queryValidPaths(paths); - - for (const auto& path : res) { - response->add_paths(path); - } - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + std::set paths; + for (const auto& path : request->paths()) { + ASSERT_INPUT_STORE_PATH(path); + paths.insert(path); + } + + auto res = store_->queryValidPaths(paths); + + for (const auto& path : res) { + response->add_paths(path); + } + + return Status::OK; + }, + __FUNCTION__); } Status QuerySubstitutablePaths(grpc::ServerContext* context, const StorePaths* request, StorePaths* response) override { - std::set paths; - for (const auto& path : request->paths()) { - ASSERT_INPUT_STORE_PATH(path); - paths.insert(path); - } - - auto res = store_->querySubstitutablePaths(paths); - - for (const auto& path : res) { - response->add_paths(path); - } - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + std::set paths; + for (const auto& path : request->paths()) { + ASSERT_INPUT_STORE_PATH(path); + paths.insert(path); + } + + auto res = store_->querySubstitutablePaths(paths); + + for (const auto& path : res) { + response->add_paths(path); + } + + return Status::OK; + }, + __FUNCTION__); } Status OptimiseStore(grpc::ServerContext* context, const google::protobuf::Empty* request, google::protobuf::Empty* response) override { - store_->optimiseStore(); - return Status::OK; + return HandleExceptions( + [&]() -> Status { + store_->optimiseStore(); + return Status::OK; + }, + __FUNCTION__); } Status VerifyStore(grpc::ServerContext* context, const nix::proto::VerifyStoreRequest* request, nix::proto::VerifyStoreResponse* response) override { - auto errors = store_->verifyStore( - request->check_contents(), static_cast(request->repair())); + return HandleExceptions( + [&]() -> Status { + auto errors = + store_->verifyStore(request->check_contents(), + static_cast(request->repair())); - response->set_errors(errors); + response->set_errors(errors); - return Status::OK; + return Status::OK; + }, + __FUNCTION__); } Status BuildDerivation( grpc::ServerContext* context, const nix::proto::BuildDerivationRequest* request, nix::proto::BuildDerivationResponse* response) override { - auto drv_path = request->drv_path().path(); - ASSERT_INPUT_STORE_PATH(drv_path); - auto drv = BasicDerivation::from_proto(&request->derivation(), *store_); - - auto build_mode = nix::BuildModeFrom(request->build_mode()); - if (!build_mode) { - return Status(grpc::StatusCode::INTERNAL, "Invalid build mode"); - } - - auto res = store_->buildDerivation(drv_path, drv, *build_mode); - - response->set_status(res.status_to_proto()); - response->set_error_message(res.errorMsg); - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + auto drv_path = request->drv_path().path(); + ASSERT_INPUT_STORE_PATH(drv_path); + auto drv = + BasicDerivation::from_proto(&request->derivation(), *store_); + + auto build_mode = nix::BuildModeFrom(request->build_mode()); + if (!build_mode) { + return Status(grpc::StatusCode::INTERNAL, "Invalid build mode"); + } + + auto res = store_->buildDerivation(drv_path, drv, *build_mode); + + response->set_status(res.status_to_proto()); + response->set_error_message(res.errorMsg); + + return Status::OK; + }, + __FUNCTION__); } Status AddSignatures(grpc::ServerContext* context, const nix::proto::AddSignaturesRequest* request, google::protobuf::Empty* response) override { - auto path = request->path().path(); - ASSERT_INPUT_STORE_PATH(path); + return HandleExceptions( + [&]() -> Status { + auto path = request->path().path(); + ASSERT_INPUT_STORE_PATH(path); - StringSet sigs; - sigs.insert(request->sigs().sigs().begin(), request->sigs().sigs().end()); + StringSet sigs; + sigs.insert(request->sigs().sigs().begin(), + request->sigs().sigs().end()); - store_->addSignatures(path, sigs); + store_->addSignatures(path, sigs); - return Status::OK; + return Status::OK; + }, + __FUNCTION__); } Status QueryMissing(grpc::ServerContext* context, const StorePaths* request, nix::proto::QueryMissingResponse* response) override { - std::set targets; - for (auto& path : request->paths()) { - ASSERT_INPUT_STORE_PATH(path); - targets.insert(path); - } - PathSet will_build; - PathSet will_substitute; - PathSet unknown; - // TODO(grfn): Switch to concrete size type - unsigned long long download_size; - unsigned long long nar_size; - - store_->queryMissing(targets, will_build, will_substitute, unknown, - download_size, nar_size); - for (auto& path : will_build) { - response->add_will_build(path); - } - for (auto& path : will_substitute) { - response->add_will_substitute(path); - } - for (auto& path : unknown) { - response->add_unknown(path); - } - response->set_download_size(download_size); - response->set_nar_size(nar_size); - - return Status::OK; + return HandleExceptions( + [&]() -> Status { + std::set targets; + for (auto& path : request->paths()) { + ASSERT_INPUT_STORE_PATH(path); + targets.insert(path); + } + PathSet will_build; + PathSet will_substitute; + PathSet unknown; + // TODO(grfn): Switch to concrete size type + unsigned long long download_size; + unsigned long long nar_size; + + store_->queryMissing(targets, will_build, will_substitute, unknown, + download_size, nar_size); + for (auto& path : will_build) { + response->add_will_build(path); + } + for (auto& path : will_substitute) { + response->add_will_substitute(path); + } + for (auto& path : unknown) { + response->add_unknown(path); + } + response->set_download_size(download_size); + response->set_nar_size(nar_size); + + return Status::OK; + }, + __FUNCTION__); }; private: + Status HandleExceptions(std::function fn, + absl::string_view methodName) { + try { + return fn(); + } catch (Unsupported& e) { + return Status(grpc::StatusCode::UNIMPLEMENTED, + absl::StrCat(methodName, " is not supported: ", e.what())); + } catch (Error& e) { + return Status(grpc::StatusCode::INTERNAL, e.what()); + } + // add more specific Error-Status mappings above + } + ref store_; }; -- cgit 1.4.1