feat(3p/nix/daemon): catch-all explicit Error-Status conversion r/1573

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 <grfn@gws.fyi>

1 files changed, 318 insertions, 230 deletions

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 <sstream>
 
+#include <absl/strings/str_cat.h>
 #include <absl/strings/str_format.h>
 #include <google/protobuf/empty.pb.h>
 #include <google/protobuf/util/time_util.h>
@@ -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<nix::proto::AddToStoreRequest>;
 
@@ -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<nix::proto::AddToStoreRequest>* 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<LocalStore>();
-    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<LocalStore>();
+          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<const char*>(&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<const char*>(&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<Path> 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<Path> 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<Path> 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<Path> 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<RepairFlag>(request->repair()));
+    return HandleExceptions(
+        [&]() -> Status {
+          auto errors =
+              store_->verifyStore(request->check_contents(),
+                                  static_cast<RepairFlag>(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<Path> 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<Path> 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<Status(void)> 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<nix::Store> store_;
 };