Improve logging on API misuse assert

include/grpcpp/impl/codegen/call_op_set.h

@@ -20,13 +20,12 @@
 #define GRPCPP_IMPL_CODEGEN_CALL_OP_SET_H
 
 #include <assert.h>
-#include <array>
 #include <cstring>
-#include <functional>
 #include <map>
 #include <memory>
-#include <vector>
 
+#include <grpc/impl/codegen/compression_types.h>
+#include <grpc/impl/codegen/grpc_types.h>
 #include <grpcpp/impl/codegen/byte_buffer.h>
 #include <grpcpp/impl/codegen/call.h>
 #include <grpcpp/impl/codegen/call_hook.h>
@@ -42,10 +41,6 @@
 #include <grpcpp/impl/codegen/slice.h>
 #include <grpcpp/impl/codegen/string_ref.h>
 
-#include <grpc/impl/codegen/atm.h>
-#include <grpc/impl/codegen/compression_types.h>
-#include <grpc/impl/codegen/grpc_types.h>
-
 namespace grpc {
 
 extern CoreCodegenInterface* g_core_codegen_interface;
@@ -940,18 +935,29 @@ class CallOpSet : public CallOpSetInterface,
     this->Op4::AddOp(ops, &nops);
     this->Op5::AddOp(ops, &nops);
     this->Op6::AddOp(ops, &nops);
-    GPR_CODEGEN_ASSERT(GRPC_CALL_OK ==
-                       g_core_codegen_interface->grpc_call_start_batch(
-                           call_.call(), ops, nops, core_cq_tag(), nullptr));
+
+    grpc_call_error err = g_core_codegen_interface->grpc_call_start_batch(
+        call_.call(), ops, nops, core_cq_tag(), nullptr);
+
+    if (err != GRPC_CALL_OK) {
+      // A failure here indicates an API misuse; for example, doing a Write
+      // while another Write is already pending on the same RPC or invoking
+      // WritesDone multiple times
+      gpr_log(GPR_ERROR, "API misuse of type %s observed",
+              g_core_codegen_interface->grpc_call_error_to_string(err));
+      GPR_CODEGEN_ASSERT(false);
+    }
   }
 
   // Should be called after interceptors are done running on the finalize result
   // path
   void ContinueFinalizeResultAfterInterception() override {
     done_intercepting_ = true;
-    GPR_CODEGEN_ASSERT(GRPC_CALL_OK ==
-                       g_core_codegen_interface->grpc_call_start_batch(
-                           call_.call(), nullptr, 0, core_cq_tag(), nullptr));
+    // The following call_start_batch is internally-generated so no need for an
+    // explanatory log on failure.
+    GPR_CODEGEN_ASSERT(g_core_codegen_interface->grpc_call_start_batch(
+                           call_.call(), nullptr, 0, core_cq_tag(), nullptr) ==
+                       GRPC_CALL_OK);
   }
 
  private:

include/grpcpp/impl/codegen/core_codegen.h

@@ -73,7 +73,8 @@ class CoreCodegen final : public CoreCodegenInterface {
                                                void* reserved) override;
   void grpc_call_ref(grpc_call* call) override;
   void grpc_call_unref(grpc_call* call) override;
-  virtual void* grpc_call_arena_alloc(grpc_call* call, size_t length) override;
+  void* grpc_call_arena_alloc(grpc_call* call, size_t length) override;
+  const char* grpc_call_error_to_string(grpc_call_error error) override;
 
   grpc_byte_buffer* grpc_byte_buffer_copy(grpc_byte_buffer* bb) override;
   void grpc_byte_buffer_destroy(grpc_byte_buffer* bb) override;

include/grpcpp/impl/codegen/core_codegen_interface.h

@@ -114,6 +114,7 @@ class CoreCodegenInterface {
   virtual void grpc_call_ref(grpc_call* call) = 0;
   virtual void grpc_call_unref(grpc_call* call) = 0;
   virtual void* grpc_call_arena_alloc(grpc_call* call, size_t length) = 0;
+  virtual const char* grpc_call_error_to_string(grpc_call_error error) = 0;
   virtual grpc_slice grpc_empty_slice() = 0;
   virtual grpc_slice grpc_slice_malloc(size_t length) = 0;
   virtual void grpc_slice_unref(grpc_slice slice) = 0;

src/cpp/common/core_codegen.cc

@@ -123,6 +123,9 @@ void CoreCodegen::grpc_call_unref(grpc_call* call) { ::grpc_call_unref(call); }
 void* CoreCodegen::grpc_call_arena_alloc(grpc_call* call, size_t length) {
   return ::grpc_call_arena_alloc(call, length);
 }
+const char* CoreCodegen::grpc_call_error_to_string(grpc_call_error error) {
+  return ::grpc_call_error_to_string(error);
+}
 
 int CoreCodegen::grpc_byte_buffer_reader_init(grpc_byte_buffer_reader* reader,
                                               grpc_byte_buffer* buffer) {

src/cpp/server/server_cc.cc

@@ -204,11 +204,13 @@ ServerInterface::RegisteredAsyncRequest::RegisteredAsyncRequest(
 void ServerInterface::RegisteredAsyncRequest::IssueRequest(
     void* registered_method, grpc_byte_buffer** payload,
     ServerCompletionQueue* notification_cq) {
-  GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_registered_call(
-                                 server_->server(), registered_method, &call_,
-                                 &context_->deadline_,
-                                 context_->client_metadata_.arr(), payload,
-                                 call_cq_->cq(), notification_cq->cq(), this));
+  // The following call_start_batch is internally-generated so no need for an
+  // explanatory log on failure.
+  GPR_ASSERT(grpc_server_request_registered_call(
+                 server_->server(), registered_method, &call_,
+                 &context_->deadline_, context_->client_metadata_.arr(),
+                 payload, call_cq_->cq(), notification_cq->cq(),
+                 this) == GRPC_CALL_OK);
 }
 
 ServerInterface::GenericAsyncRequest::GenericAsyncRequest(
@@ -220,10 +222,12 @@ ServerInterface::GenericAsyncRequest::GenericAsyncRequest(
   grpc_call_details_init(&call_details_);
   GPR_ASSERT(notification_cq);
   GPR_ASSERT(call_cq);
-  GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(
-                                 server->server(), &call_, &call_details_,
-                                 context->client_metadata_.arr(), call_cq->cq(),
-                                 notification_cq->cq(), this));
+  // The following call_start_batch is internally-generated so no need for an
+  // explanatory log on failure.
+  GPR_ASSERT(grpc_server_request_call(server->server(), &call_, &call_details_,
+                                      context->client_metadata_.arr(),
+                                      call_cq->cq(), notification_cq->cq(),
+                                      this) == GRPC_CALL_OK);
 }
 
 bool ServerInterface::GenericAsyncRequest::FinalizeResult(void** tag,
@@ -571,12 +575,11 @@ class Server::CallbackRequest final : public Server::CallbackRequestBase {
 
   bool Request() override {
     if (method_tag_) {
-      if (GRPC_CALL_OK !=
-          grpc_server_request_registered_call(
+      if (grpc_server_request_registered_call(
               server_->c_server(), method_tag_, &call_, &deadline_,
               &request_metadata_,
               has_request_payload_ ? &request_payload_ : nullptr, cq_->cq(),
-              cq_->cq(), static_cast<void*>(&tag_))) {
+              cq_->cq(), static_cast<void*>(&tag_)) != GRPC_CALL_OK) {
         return false;
       }
     } else {

src/cpp/server/server_context.cc

@@ -17,7 +17,6 @@
  */
 
 #include <grpcpp/impl/codegen/server_context_impl.h>
-#include <grpcpp/support/server_callback.h>
 
 #include <algorithm>
 #include <mutex>
@@ -30,6 +29,7 @@
 #include <grpc/support/log.h>
 #include <grpcpp/impl/call.h>
 #include <grpcpp/impl/codegen/completion_queue_impl.h>
+#include <grpcpp/support/server_callback.h>
 #include <grpcpp/support/time.h>
 
 #include "src/core/lib/gprpp/ref_counted.h"
@@ -123,7 +123,7 @@ class ServerContext::CompletionOp final
   // RPC. This should set hijacking state for each of the ops.
   void SetHijackingState() override {
     /* Servers don't allow hijacking */
-    GPR_CODEGEN_ASSERT(false);
+    GPR_ASSERT(false);
   }
 
   /* Should be called after interceptors are done running */
@@ -139,9 +139,8 @@ class ServerContext::CompletionOp final
       return;
     }
     /* Start a dummy op so that we can return the tag */
-    GPR_CODEGEN_ASSERT(
-        GRPC_CALL_OK ==
-        grpc_call_start_batch(call_.call(), nullptr, 0, core_cq_tag_, nullptr));
+    GPR_ASSERT(grpc_call_start_batch(call_.call(), nullptr, 0, core_cq_tag_,
+                                     nullptr) == GRPC_CALL_OK);
   }
 
  private:
@@ -181,8 +180,10 @@ void ServerContext::CompletionOp::FillOps(::grpc::internal::Call* call) {
   interceptor_methods_.SetCall(&call_);
   interceptor_methods_.SetReverse();
   interceptor_methods_.SetCallOpSetInterface(this);
-  GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(call->call(), &ops, 1,
-                                                   core_cq_tag_, nullptr));
+  // The following call_start_batch is internally-generated so no need for an
+  // explanatory log on failure.
+  GPR_ASSERT(grpc_call_start_batch(call->call(), &ops, 1, core_cq_tag_,
+                                   nullptr) == GRPC_CALL_OK);
   /* No interceptors to run here */
 }