Automatically return gRPC errors for framework-level errors

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changed
+
+- `ccf::RpcContext::set_response()` has been renamed to `ccf::RpcContext::set_response_json()` (#4813).
+
 ## [4.0.0-dev3]
 
 [4.0.0-dev3]: https://github.com/microsoft/CCF/releases/tag/ccf-4.0.0-dev3

include/ccf/rpc_context.h

@@ -149,44 +149,21 @@ namespace ccf
       set_response_trailer(kv.first, kv.second);
     }
 
-    /// Construct OData-formatted response to capture multiple error details
+    virtual void set_response_json(
+      nlohmann::json& body, http_status status) = 0;
+
+    /// Construct error response, formatted according to the request content
+    /// type (either JSON OData-formatted or gRPC error)
     virtual void set_error(
       http_status status,
       const std::string& code,
       std::string&& msg,
-      std::vector<ccf::ODataErrorDetails>& details)
-    {
-      nlohmann::json body =
-        ccf::ODataErrorResponse{ccf::ODataError{code, std::move(msg), details}};
-      set_response(body, status);
-    }
+      const std::vector<ccf::ODataErrorDetails>& details = {}) = 0;
 
-    /// Construct OData-formatted error response.
-    virtual void set_error(
-      http_status status, const std::string& code, std::string&& msg)
-    {
-      set_error(ccf::ErrorDetails{status, code, std::move(msg)});
-    }
+    /// Construct error response, formatted according to the request content
+    /// type (either JSON OData-formatted or gRPC error)
+    virtual void set_error(ccf::ErrorDetails&& error) = 0;
 
-    /// Construct OData-formatted error response.
-    virtual void set_error(ccf::ErrorDetails&& error)
-    {
-      nlohmann::json body = ccf::ODataErrorResponse{
-        ccf::ODataError{std::move(error.code), std::move(error.msg)}};
-      set_response(body, error.status);
-    }
-
-    virtual void set_response(nlohmann::json& body, http_status status)
-    {
-      // Set error_handler to replace, to avoid throwing if the error message
-      // contains non-UTF8 characters. Other args are default values
-      const auto s =
-        body.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace);
-      set_response_status(status);
-      set_response_body(std::vector<uint8_t>(s.begin(), s.end()));
-      set_response_header(
-        http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON);
-    }
     ///@}
 
     /// \defgroup Framework metadata

samples/apps/logging/logging.cpp

@@ -515,7 +515,7 @@ namespace loggingapp
             },
           }};
 
-          ctx.rpc_ctx->set_response(response, HTTP_STATUS_BAD_REQUEST);
+          ctx.rpc_ctx->set_response_json(response, HTTP_STATUS_BAD_REQUEST);
           return;
         }
 
@@ -524,7 +524,7 @@ namespace loggingapp
         if (record.has_value())
         {
           nlohmann::json response = LoggingGet::Out{record.value()};
-          ctx.rpc_ctx->set_response(response, HTTP_STATUS_OK);
+          ctx.rpc_ctx->set_response_json(response, HTTP_STATUS_OK);
           return;
         }
 
@@ -537,7 +537,7 @@ namespace loggingapp
           },
         }};
 
-        ctx.rpc_ctx->set_response(response, HTTP_STATUS_BAD_REQUEST);
+        ctx.rpc_ctx->set_response_json(response, HTTP_STATUS_BAD_REQUEST);
       };
 
       make_read_only_endpoint(

src/endpoints/grpc/grpc_status.h

@@ -0,0 +1,114 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the Apache 2.0 License.
+#pragma once
+
+#include "ccf/http_consts.h"
+#include "ccf/http_header_map.h"
+
+#include <string>
+
+#define GRPC_STATUS_MAP(XX) \
+  XX(0, OK, "Ok") \
+  XX(1, CANCELLED, "Cancelled") \
+  XX(2, UNKNOWN, "Unknown") \
+  XX(3, INVALID_ARGUMENT, "Invalid Argument") \
+  XX(4, DEADLINE_EXCEEDED, "Deadline Exceeded") \
+  XX(5, NOT_FOUND, "Not Found") \
+  XX(6, ALREADY_EXISTS, "Already Exists") \
+  XX(7, PERMISSION_DENIED, "Permission Denied") \
+  XX(8, RESOURCE_EXHAUSTED, "Resource Exhausted") \
+  XX(9, FAILED_PRECONDITION, "Failed Precondition") \
+  XX(10, ABORTED, "Aborted") \
+  XX(11, OUT_OF_RANGE, "Out Of Range") \
+  XX(12, UNIMPLEMENTED, "Unimplemented") \
+  XX(13, INTERNAL, "Internal") \
+  XX(14, UNAVAILABLE, "Unavailable") \
+  XX(15, DATA_LOSS, "Data Loss") \
+  XX(16, UNAUTHENTICATED, "Unauthenticated")
+
+enum grpc_status
+{
+#define XX(num, name, string) GRPC_STATUS_##name = num,
+  GRPC_STATUS_MAP(XX)
+#undef XX
+};
+
+static inline const char* grpc_status_str(enum grpc_status s)
+{
+  switch (s)
+  {
+#define XX(num, name, string) \
+  case GRPC_STATUS_##name: \
+    return string;
+    GRPC_STATUS_MAP(XX)
+#undef XX
+    default:
+      return "<unknown>";
+  }
+}
+
+// CCF is primarily an HTTP framework. However, gRPC clients should be returned
+// an appropriate gRPC status code when an error is returned at the framework
+// level (e.g. authentication error) so we use the following function to
+// automatically convert HTTP errors to gRPC statuses.
+// Inspired by
+// https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto
+static grpc_status http_status_to_grpc(enum http_status s)
+{
+  // Note: GRPC_STATUS_CANCELLED, GRPC_STATUS_ABORTED and GRPC_STATUS_DATA_LOSS
+  // are currently never returned
+  switch (s)
+  {
+    case HTTP_STATUS_UNAUTHORIZED: // 401
+      return GRPC_STATUS_UNAUTHENTICATED;
+    case HTTP_STATUS_FORBIDDEN: // 404
+      return GRPC_STATUS_PERMISSION_DENIED;
+    case HTTP_STATUS_NOT_FOUND: // 404
+      return GRPC_STATUS_NOT_FOUND;
+    case HTTP_STATUS_CONFLICT: // 409
+      return GRPC_STATUS_ALREADY_EXISTS;
+    case HTTP_STATUS_PRECONDITION_FAILED: // 412
+      return GRPC_STATUS_FAILED_PRECONDITION;
+    case HTTP_STATUS_RANGE_NOT_SATISFIABLE: // 416
+      return GRPC_STATUS_OUT_OF_RANGE;
+    case HTTP_STATUS_TOO_MANY_REQUESTS: // 429
+      return GRPC_STATUS_RESOURCE_EXHAUSTED;
+    case HTTP_STATUS_NOT_IMPLEMENTED: // 501
+      return GRPC_STATUS_UNIMPLEMENTED;
+    case HTTP_STATUS_SERVICE_UNAVAILABLE: // 503
+      return GRPC_STATUS_UNAVAILABLE;
+    case HTTP_STATUS_GATEWAY_TIMEOUT: // 504
+      return GRPC_STATUS_DEADLINE_EXCEEDED;
+    default:
+    {
+      // For non-specific codes, we approximate to the closest generic code
+      if (s >= 200 && s < 300) // 2xx
+        return GRPC_STATUS_OK;
+      else if (s >= 400 && s < 500) // 4xx
+        return GRPC_STATUS_INVALID_ARGUMENT;
+      else if (s >= 500) // 5xx
+        return GRPC_STATUS_INTERNAL;
+      else
+        return GRPC_STATUS_UNKNOWN;
+    }
+  }
+}
+
+namespace ccf::grpc
+{
+  static const http::HeaderMap default_response_headers = {
+    {http::headers::CONTENT_TYPE, http::headervalues::contenttype::GRPC}};
+
+  static constexpr auto TRAILER_STATUS = "grpc-status";
+  static constexpr auto TRAILER_MESSAGE = "grpc-message";
+
+  static http::HeaderKeyValue make_status_trailer(int32_t code)
+  {
+    return {TRAILER_STATUS, std::to_string(code)};
+  }
+
+  static http::HeaderKeyValue make_message_trailer(const std::string& msg)
+  {
+    return {TRAILER_MESSAGE, msg};
+  }
+}

src/endpoints/grpc/status.h

@@ -2,61 +2,20 @@
 // Licensed under the Apache 2.0 License.
 #pragma once
 
+#include "grpc_status.h"
 #include "status.pb.h"
 
 #include <optional>
 #include <string>
 
-// Mapping to HTTP errors as per
-// https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto
-#define GRPC_STATUS_MAP(XX) \
-  XX(0, OK, "Ok", 200) \
-  XX(1, CANCELLED, "Cancelled", 499) \
-  XX(2, UNKNOWN, "Unknown", 500) \
-  XX(3, INVALID_ARGUMENT, "Invalid Argument", 400) \
-  XX(4, DEADLINE_EXCEEDED, "Deadline Exceeded", 504) \
-  XX(5, NOT_FOUND, "Not Found", 404) \
-  XX(6, ALREADY_EXISTS, "Already Exists", 409) \
-  XX(7, PERMISSION_DENIED, "Permission Denied", 403) \
-  XX(8, RESOURCE_EXHAUSTED, "Resource Exhausted", 429) \
-  XX(9, FAILED_PRECONDITION, "Failed Precondition", 400) \
-  XX(10, ABORTED, "Aborted", 409) \
-  XX(11, OUT_OF_RANGE, "Out Of Range", 400) \
-  XX(12, UNIMPLEMENTED, "Unimplemented", 501) \
-  XX(13, INTERNAL, "Internal", 500) \
-  XX(14, UNAVAILABLE, "Unavailable", 503) \
-  XX(15, DATA_LOSS, "Data Loss", 500) \
-  XX(16, UNAUTHENTICATED, "Unauthenticated", 401)
-
-enum grpc_status
-{
-#define XX(num, name, string, http_eq) GRPC_STATUS_##name = num,
-  GRPC_STATUS_MAP(XX)
-#undef XX
-};
-
-static inline const char* grpc_status_str(enum grpc_status s)
-{
-  switch (s)
-  {
-#define XX(num, name, string, http_eq) \
-  case GRPC_STATUS_##name: \
-    return string;
-    GRPC_STATUS_MAP(XX)
-#undef XX
-    default:
-      return "<unknown>";
-  }
-}
-
 namespace ccf::grpc
 {
-  int32_t status_to_code(const grpc_status& status)
+  static int32_t status_to_code(const grpc_status& status)
   {
     return static_cast<int32_t>(status);
   }
 
-  protobuf::Status make_grpc_status(
+  static protobuf::Status make_grpc_status(
     enum grpc_status status,
     const std::optional<std::string>& msg = std::nullopt)
   {
@@ -81,7 +40,7 @@ namespace ccf::grpc
     return s;
   }
 
-  protobuf::Status make_grpc_status_ok()
+  static protobuf::Status make_grpc_status_ok()
   {
     return make_grpc_status(GRPC_STATUS_OK);
   }

src/endpoints/grpc/types.h

@@ -13,22 +13,6 @@
 
 namespace ccf::grpc
 {
-  static const http::HeaderMap default_response_headers = {
-    {http::headers::CONTENT_TYPE, http::headervalues::contenttype::GRPC}};
-
-  static constexpr auto TRAILER_STATUS = "grpc-status";
-  static constexpr auto TRAILER_MESSAGE = "grpc-message";
-
-  static http::HeaderKeyValue make_status_trailer(int32_t code)
-  {
-    return {TRAILER_STATUS, std::to_string(code)};
-  }
-
-  static http::HeaderKeyValue make_message_trailer(const std::string& msg)
-  {
-    return {TRAILER_MESSAGE, msg};
-  }
-
   template <typename T>
   struct SuccessResponse
   {
@@ -62,28 +46,29 @@ namespace ccf::grpc
   using GrpcAdapterEmptyResponse = GrpcAdapterResponse<EmptyResponse>;
 
   template <typename T>
-  GrpcAdapterResponse<T> make_success(const T& t)
+  static GrpcAdapterResponse<T> make_success(const T& t)
   {
     return SuccessResponse(t, make_grpc_status_ok());
   }
 
-  GrpcAdapterEmptyResponse make_success()
+  static GrpcAdapterEmptyResponse make_success()
   {
     return SuccessResponse(EmptyResponse{}, make_grpc_status_ok());
   }
 
-  PendingResponse make_pending()
+  static PendingResponse make_pending()
   {
     return PendingResponse{};
   }
 
-  ErrorResponse make_error(grpc_status code, const std::string& msg)
+  static ErrorResponse make_error(grpc_status code, const std::string& msg)
   {
     return ErrorResponse(make_grpc_status(code, msg));
   }
 
   template <typename T>
-  GrpcAdapterResponse<T> make_error(grpc_status code, const std::string& msg)
+  static GrpcAdapterResponse<T> make_error(
+    grpc_status code, const std::string& msg)
   {
     return ErrorResponse(make_grpc_status(code, msg));
   }

src/node/rpc/frontend.h

@@ -14,6 +14,7 @@
 #include "common/configuration.h"
 #include "consensus/aft/request.h"
 #include "enclave/rpc_handler.h"
+#include "endpoints/grpc/grpc_status.h"
 #include "forwarder.h"
 #include "http/http_jwt.h"
 #include "kv/compacted_version_conflict.h"
@@ -271,7 +272,7 @@ namespace ccf
         ctx->set_error(
           HTTP_STATUS_UNAUTHORIZED,
           ccf::errors::InvalidAuthenticationInfo,
-          "Invalid info",
+          "Invalid authentication credentials.",
           error_details);
         update_metrics(ctx);
       }
@@ -481,7 +482,7 @@ namespace ccf
           {
             LOG_FAIL_FMT(
               "Bad endpoint: During execution of {} {}, returned a non-pending "
-              "response but stole owneship of Tx object",
+              "response but stole ownership of Tx object",
               ctx->get_request_verb().c_str(),
               ctx->get_request_path());