Merge pull request #10524 from yang-g/error_details

Support C++ (binary) error_details

include/grpc++/impl/codegen/call.h

@@ -63,21 +63,31 @@ class CallHook;
 class CompletionQueue;
 extern CoreCodegenInterface* g_core_codegen_interface;
 
+const char kBinaryErrorDetailsKey[] = "grpc-status-details-bin";
+
 // TODO(yangg) if the map is changed before we send, the pointers will be a
 // mess. Make sure it does not happen.
 inline grpc_metadata* FillMetadataArray(
-    const std::multimap<grpc::string, grpc::string>& metadata) {
-  if (metadata.empty()) {
+    const std::multimap<grpc::string, grpc::string>& metadata,
+    size_t* metadata_count, const grpc::string& optional_error_details) {
+  *metadata_count = metadata.size() + (optional_error_details.empty() ? 0 : 1);
+  if (*metadata_count == 0) {
     return nullptr;
   }
   grpc_metadata* metadata_array =
       (grpc_metadata*)(g_core_codegen_interface->gpr_malloc(
-          metadata.size() * sizeof(grpc_metadata)));
+          (*metadata_count) * sizeof(grpc_metadata)));
   size_t i = 0;
   for (auto iter = metadata.cbegin(); iter != metadata.cend(); ++iter, ++i) {
     metadata_array[i].key = SliceReferencingString(iter->first);
     metadata_array[i].value = SliceReferencingString(iter->second);
   }
+  if (!optional_error_details.empty()) {
+    metadata_array[i].key =
+        g_core_codegen_interface->grpc_slice_from_static_buffer(
+            kBinaryErrorDetailsKey, sizeof(kBinaryErrorDetailsKey) - 1);
+    metadata_array[i].value = SliceReferencingString(optional_error_details);
+  }
   return metadata_array;
 }
 
@@ -216,8 +226,8 @@ class CallOpSendInitialMetadata {
     maybe_compression_level_.is_set = false;
     send_ = true;
     flags_ = flags;
-    initial_metadata_count_ = metadata.size();
-    initial_metadata_ = FillMetadataArray(metadata);
+    initial_metadata_ =
+        FillMetadataArray(metadata, &initial_metadata_count_, "");
   }
 
   void set_compression_level(grpc_compression_level level) {
@@ -454,11 +464,12 @@ class CallOpServerSendStatus {
   void ServerSendStatus(
       const std::multimap<grpc::string, grpc::string>& trailing_metadata,
       const Status& status) {
-    trailing_metadata_count_ = trailing_metadata.size();
-    trailing_metadata_ = FillMetadataArray(trailing_metadata);
+    send_error_details_ = status.error_details();
+    trailing_metadata_ = FillMetadataArray(
+        trailing_metadata, &trailing_metadata_count_, send_error_details_);
     send_status_available_ = true;
     send_status_code_ = static_cast<grpc_status_code>(GetCanonicalCode(status));
-    send_status_details_ = status.error_message();
+    send_error_message_ = status.error_message();
   }
 
  protected:
@@ -470,9 +481,9 @@ class CallOpServerSendStatus {
         trailing_metadata_count_;
     op->data.send_status_from_server.trailing_metadata = trailing_metadata_;
     op->data.send_status_from_server.status = send_status_code_;
-    status_details_slice_ = SliceReferencingString(send_status_details_);
+    error_message_slice_ = SliceReferencingString(send_error_message_);
     op->data.send_status_from_server.status_details =
-        send_status_details_.empty() ? nullptr : &status_details_slice_;
+        send_error_message_.empty() ? nullptr : &error_message_slice_;
     op->flags = 0;
     op->reserved = NULL;
   }
@@ -486,10 +497,11 @@ class CallOpServerSendStatus {
  private:
   bool send_status_available_;
   grpc_status_code send_status_code_;
-  grpc::string send_status_details_;
+  grpc::string send_error_details_;
+  grpc::string send_error_message_;
   size_t trailing_metadata_count_;
   grpc_metadata* trailing_metadata_;
-  grpc_slice status_details_slice_;
+  grpc_slice error_message_slice_;
 };
 
 class CallOpRecvInitialMetadata {
@@ -528,7 +540,7 @@ class CallOpClientRecvStatus {
   void ClientRecvStatus(ClientContext* context, Status* status) {
     metadata_map_ = &context->trailing_metadata_;
     recv_status_ = status;
-    status_details_ = g_core_codegen_interface->grpc_empty_slice();
+    error_message_ = g_core_codegen_interface->grpc_empty_slice();
   }
 
  protected:
@@ -538,7 +550,7 @@ class CallOpClientRecvStatus {
     op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;
     op->data.recv_status_on_client.trailing_metadata = metadata_map_->arr();
     op->data.recv_status_on_client.status = &status_code_;
-    op->data.recv_status_on_client.status_details = &status_details_;
+    op->data.recv_status_on_client.status_details = &error_message_;
     op->flags = 0;
     op->reserved = NULL;
   }
@@ -546,10 +558,17 @@ class CallOpClientRecvStatus {
   void FinishOp(bool* status) {
     if (recv_status_ == nullptr) return;
     metadata_map_->FillMap();
+    grpc::string binary_error_details;
+    auto iter = metadata_map_->map()->find(kBinaryErrorDetailsKey);
+    if (iter != metadata_map_->map()->end()) {
+      binary_error_details =
+          grpc::string(iter->second.begin(), iter->second.length());
+    }
     *recv_status_ = Status(static_cast<StatusCode>(status_code_),
-                           grpc::string(GRPC_SLICE_START_PTR(status_details_),
-                                        GRPC_SLICE_END_PTR(status_details_)));
-    g_core_codegen_interface->grpc_slice_unref(status_details_);
+                           grpc::string(GRPC_SLICE_START_PTR(error_message_),
+                                        GRPC_SLICE_END_PTR(error_message_)),
+                           binary_error_details);
+    g_core_codegen_interface->grpc_slice_unref(error_message_);
     recv_status_ = nullptr;
   }
 
@@ -557,7 +576,7 @@ class CallOpClientRecvStatus {
   MetadataMap* metadata_map_;
   Status* recv_status_;
   grpc_status_code status_code_;
-  grpc_slice status_details_;
+  grpc_slice error_message_;
 };
 
 /// An abstract collection of CallOpSet's, to be used whenever

include/grpc++/impl/codegen/status.h

@@ -47,10 +47,16 @@ class Status {
   /// Construct an OK instance.
   Status() : code_(StatusCode::OK) {}
 
-  /// Construct an instance with associated \a code and \a details (also
-  // referred to as "error_message").
-  Status(StatusCode code, const grpc::string& details)
-      : code_(code), details_(details) {}
+  /// Construct an instance with associated \a code and \a error_message
+  Status(StatusCode code, const grpc::string& error_message)
+      : code_(code), error_message_(error_message) {}
+
+  /// Construct an instance with \a code,  \a error_message and \a error_details
+  Status(StatusCode code, const grpc::string& error_message,
+         const grpc::string error_details)
+      : code_(code),
+        error_message_(error_message),
+        binary_error_details_(error_details) {}
 
   // Pre-defined special status objects.
   /// An OK pre-defined instance.
@@ -61,14 +67,18 @@ class Status {
   /// Return the instance's error code.
   StatusCode error_code() const { return code_; }
   /// Return the instance's error message.
-  grpc::string error_message() const { return details_; }
+  grpc::string error_message() const { return error_message_; }
+  /// Return the (binary) error details.
+  // Usually it contains a serialized google.rpc.Status proto.
+  grpc::string error_details() const { return binary_error_details_; }
 
   /// Is the status OK?
   bool ok() const { return code_ == StatusCode::OK; }
 
  private:
   StatusCode code_;
-  grpc::string details_;
+  grpc::string error_message_;
+  grpc::string binary_error_details_;
 };
 
 }  // namespace grpc

src/proto/grpc/testing/echo_messages.proto

@@ -38,6 +38,13 @@ message DebugInfo {
   string detail = 2;
 }
 
+// Error status client expects to see.
+message ErrorStatus {
+  int32 code = 1;
+  string error_message = 2;
+  string binary_error_details = 3;
+}
+
 message RequestParams {
   bool echo_deadline = 1;
   int32 client_cancel_after_us = 2;
@@ -51,6 +58,8 @@ message RequestParams {
   string expected_transport_security_type = 10;
   DebugInfo debug_info = 11;
   bool server_die = 12; // Server should not see a request with this set.
+  string binary_error_details = 13;
+  ErrorStatus expected_error = 14;
 }
 
 message EchoRequest {

test/cpp/end2end/end2end_test.cc

@@ -1130,6 +1130,39 @@ TEST_P(End2endTest, BinaryTrailerTest) {
   EXPECT_TRUE(returned_info.ParseFromString(ToString(iter->second)));
 }
 
+TEST_P(End2endTest, ExpectErrorTest) {
+  ResetStub();
+
+  std::vector<ErrorStatus> expected_status;
+  expected_status.emplace_back();
+  expected_status.back().set_code(13);  // INTERNAL
+  expected_status.back().set_error_message("text error message");
+  expected_status.back().set_binary_error_details("text error details");
+  expected_status.emplace_back();
+  expected_status.back().set_code(13);  // INTERNAL
+  expected_status.back().set_error_message("text error message");
+  expected_status.back().set_binary_error_details(
+      "\x0\x1\x2\x3\x4\x5\x6\x8\x9\xA\xB");
+
+  for (auto iter = expected_status.begin(); iter != expected_status.end();
+       ++iter) {
+    EchoRequest request;
+    EchoResponse response;
+    ClientContext context;
+    request.set_message("Hello");
+    auto* error = request.mutable_param()->mutable_expected_error();
+    error->set_code(iter->code());
+    error->set_error_message(iter->error_message());
+    error->set_binary_error_details(iter->binary_error_details());
+
+    Status s = stub_->Echo(&context, request, &response);
+    EXPECT_FALSE(s.ok());
+    EXPECT_EQ(iter->code(), s.error_code());
+    EXPECT_EQ(iter->error_message(), s.error_message());
+    EXPECT_EQ(iter->binary_error_details(), s.error_details());
+  }
+}
+
 //////////////////////////////////////////////////////////////////////////
 // Test with and without a proxy.
 class ProxyEnd2endTest : public End2endTest {

test/cpp/end2end/test_service_impl.cc

@@ -92,6 +92,11 @@ Status TestServiceImpl::Echo(ServerContext* context, const EchoRequest* request,
     gpr_log(GPR_ERROR, "The request should not reach application handler.");
     GPR_ASSERT(0);
   }
+  if (request->has_param() && request->param().has_expected_error()) {
+    const auto& error = request->param().expected_error();
+    return Status(static_cast<StatusCode>(error.code()), error.error_message(),
+                  error.binary_error_details());
+  }
   int server_try_cancel = GetIntValueFromMetadata(
       kServerTryCancelRequest, context->client_metadata(), DO_NOT_CANCEL);
   if (server_try_cancel > DO_NOT_CANCEL) {