PR Feedback Changes

src/core/ext/filters/http/server/http_server_filter.cc

@@ -321,17 +321,8 @@ static void hs_recv_message_ready(void* user_data, grpc_error* err) {
 static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) {
   grpc_call_element* elem = static_cast<grpc_call_element*>(user_data);
   call_data* calld = static_cast<call_data*>(elem->call_data);
-  if (calld->recv_initial_metadata_ready_error != GRPC_ERROR_NONE) {
+  err =
-    if (err == GRPC_ERROR_NONE) {
+      grpc_error_maybe_add_child(err, calld->recv_initial_metadata_ready_error);
-      err = GRPC_ERROR_REF(calld->recv_initial_metadata_ready_error);
-    } else if (err != calld->recv_initial_metadata_ready_error) {
-      err = grpc_error_add_child(err, calld->recv_initial_metadata_ready_error);
-    } else {
-      err = GRPC_ERROR_REF(err);
-    }
-  } else {
-    err = GRPC_ERROR_REF(err);
-  }
   GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err);
 }
 
@@ -382,6 +373,8 @@ static grpc_error* hs_mutate_op(grpc_call_element* elem,
   if (op->recv_trailing_metadata) {
     calld->original_recv_trailing_metadata_ready =
         op->payload->recv_trailing_metadata.recv_trailing_metadata_ready;
+    op->payload->recv_trailing_metadata.recv_trailing_metadata_ready =
+        &calld->recv_trailing_metadata_ready;
   }
 
   if (op->send_trailing_metadata) {

src/core/ext/filters/message_size/message_size_filter.cc

@@ -155,17 +155,7 @@ static void recv_message_ready(void* user_data, grpc_error* error) {
 static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) {
   grpc_call_element* elem = static_cast<grpc_call_element*>(user_data);
   call_data* calld = static_cast<call_data*>(elem->call_data);
-  if (calld->error != GRPC_ERROR_NONE) {
+  error = grpc_error_maybe_add_child(error, calld->error);
-    if (error == GRPC_ERROR_NONE) {
-      error = GRPC_ERROR_REF(calld->error);
-    } else if (error != calld->error) {
-      error = grpc_error_add_child(error, GRPC_ERROR_REF(calld->error));
-    } else {
-      error = GRPC_ERROR_REF(error);
-    }
-  } else {
-    error = GRPC_ERROR_REF(error);
-  }
   // Invoke the next callback.
   GRPC_CLOSURE_RUN(calld->next_recv_trailing_metadata_ready, error);
 }

src/core/lib/iomgr/error.cc

@@ -518,6 +518,20 @@ grpc_error* grpc_error_add_child(grpc_error* src, grpc_error* child) {
   return new_err;
 }
 
+grpc_error* grpc_error_maybe_add_child(grpc_error* src, grpc_error* child) {
+  if (src != GRPC_ERROR_NONE) {
+    if (child == GRPC_ERROR_NONE) {
+      return GRPC_ERROR_REF(src);
+    } else if (child != src) {
+      return grpc_error_add_child(src, GRPC_ERROR_REF(child));
+    } else {
+      return GRPC_ERROR_REF(src);
+    }
+  } else {
+    return GRPC_ERROR_REF(child);
+  }
+}
+
 static const char* no_error_string = "\"No Error\"";
 static const char* oom_error_string = "\"Out of memory\"";
 static const char* cancelled_error_string = "\"Cancelled\"";

src/core/lib/iomgr/error.h

@@ -187,6 +187,13 @@ bool grpc_error_get_str(grpc_error* error, grpc_error_strs which,
 /// child error.
 grpc_error* grpc_error_add_child(grpc_error* src,
                                  grpc_error* child) GRPC_MUST_USE_RESULT;
+/// Produce an error that is a combination of both src and child.
+//  If src or child, is GRPC_ERROR_NONE, a new reference to the other error is
+//  returned. Otherwise, a new error with src as the parent and child as the
+//  child is returned.
+grpc_error* grpc_error_maybe_add_child(grpc_error* src,
+                                       grpc_error* child) GRPC_MUST_USE_RESULT;
+
 grpc_error* grpc_os_error(const char* file, int line, int err,
                           const char* call_name) GRPC_MUST_USE_RESULT;
 

src/core/lib/security/transport/server_auth_filter.cc

@@ -41,6 +41,9 @@ struct call_data {
   grpc_transport_stream_op_batch* recv_initial_metadata_batch;
   grpc_closure* original_recv_initial_metadata_ready;
   grpc_closure recv_initial_metadata_ready;
+  grpc_error* error;
+  grpc_closure recv_trailing_metadata_ready;
+  grpc_closure* original_recv_trailing_metadata_ready;
   grpc_metadata_array md;
   const grpc_metadata* consumed_md;
   size_t num_consumed_md;
@@ -111,6 +114,7 @@ static void on_md_processing_done_inner(grpc_call_element* elem,
         batch->payload->recv_initial_metadata.recv_initial_metadata,
         remove_consumed_md, elem, "Response metadata filtering error");
   }
+  calld->error = GRPC_ERROR_REF(error);
   GRPC_CLOSURE_SCHED(calld->original_recv_initial_metadata_ready, error);
 }
 
@@ -186,6 +190,13 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) {
                    GRPC_ERROR_REF(error));
 }
 
+static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) {
+  grpc_call_element* elem = static_cast<grpc_call_element*>(user_data);
+  call_data* calld = static_cast<call_data*>(elem->call_data);
+  err = grpc_error_maybe_add_child(err, calld->error);
+  GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err);
+}
+
 static void auth_start_transport_stream_op_batch(
     grpc_call_element* elem, grpc_transport_stream_op_batch* batch) {
   call_data* calld = static_cast<call_data*>(elem->call_data);
@@ -197,6 +208,12 @@ static void auth_start_transport_stream_op_batch(
     batch->payload->recv_initial_metadata.recv_initial_metadata_ready =
         &calld->recv_initial_metadata_ready;
   }
+  if (batch->recv_trailing_metadata) {
+    calld->original_recv_trailing_metadata_ready =
+        batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready;
+    batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready =
+        &calld->recv_trailing_metadata_ready;
+  }
   grpc_call_next_op(elem, batch);
 }
 
@@ -210,6 +227,9 @@ static grpc_error* init_call_elem(grpc_call_element* elem,
   GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready,
                     recv_initial_metadata_ready, elem,
                     grpc_schedule_on_exec_ctx);
+  GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready,
+                    recv_trailing_metadata_ready, elem,
+                    grpc_schedule_on_exec_ctx);
   // Create server security context.  Set its auth context from channel
   // data and save it in the call context.
   grpc_server_security_context* server_ctx =
@@ -229,7 +249,10 @@ static grpc_error* init_call_elem(grpc_call_element* elem,
 /* Destructor for call_data */
 static void destroy_call_elem(grpc_call_element* elem,
                               const grpc_call_final_info* final_info,
-                              grpc_closure* ignored) {}
+                              grpc_closure* ignored) {
+  call_data* calld = static_cast<call_data*>(elem->call_data);
+  GRPC_ERROR_UNREF(calld->error);
+}
 
 /* Constructor for channel_data */
 static grpc_error* init_channel_elem(grpc_channel_element* elem,

src/core/lib/surface/call.cc

@@ -512,7 +512,7 @@ static void destroy_call(void* call, grpc_error* error) {
   grpc_slice slice = grpc_empty_slice();
   grpc_error_get_status(c->status_error, c->send_deadline,
                         &c->final_info.final_status, &slice, nullptr,
-                        &c->final_info.error_string);
+                        &(c->final_info.error_string));
   GRPC_ERROR_UNREF(c->status_error);
   c->final_info.stats.latency =
       gpr_time_sub(gpr_now(GPR_CLOCK_MONOTONIC), c->start_time);

src/core/lib/surface/server.cc

@@ -149,6 +149,9 @@ struct call_data {
   grpc_closure server_on_recv_initial_metadata;
   grpc_closure kill_zombie_closure;
   grpc_closure* on_done_recv_initial_metadata;
+  grpc_closure recv_trailing_metadata_ready;
+  grpc_error* error;
+  grpc_closure* original_recv_trailing_metadata_ready;
 
   grpc_closure publish;
 
@@ -730,6 +733,14 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) {
   GRPC_CLOSURE_RUN(calld->on_done_recv_initial_metadata, error);
 }
 
+static void server_recv_trailing_metadata_ready(void* user_data,
+                                                grpc_error* err) {
+  grpc_call_element* elem = static_cast<grpc_call_element*>(user_data);
+  call_data* calld = static_cast<call_data*>(elem->call_data);
+  err = grpc_error_maybe_add_child(err, calld->error);
+  GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err);
+}
+
 static void server_mutate_op(grpc_call_element* elem,
                              grpc_transport_stream_op_batch* op) {
   call_data* calld = static_cast<call_data*>(elem->call_data);
@@ -745,6 +756,12 @@ static void server_mutate_op(grpc_call_element* elem,
     op->payload->recv_initial_metadata.recv_flags =
         &calld->recv_initial_metadata_flags;
   }
+  if (op->recv_trailing_metadata) {
+    calld->original_recv_trailing_metadata_ready =
+        op->payload->recv_trailing_metadata.recv_trailing_metadata_ready;
+    op->payload->recv_trailing_metadata.recv_trailing_metadata_ready =
+        &calld->recv_trailing_metadata_ready;
+  }
 }
 
 static void server_start_transport_stream_op_batch(
@@ -828,7 +845,9 @@ static grpc_error* init_call_elem(grpc_call_element* elem,
   GRPC_CLOSURE_INIT(&calld->server_on_recv_initial_metadata,
                     server_on_recv_initial_metadata, elem,
                     grpc_schedule_on_exec_ctx);
-
+  GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready,
+                    server_recv_trailing_metadata_ready, elem,
+                    grpc_schedule_on_exec_ctx);
   server_ref(chand->server);
   return GRPC_ERROR_NONE;
 }
@@ -840,7 +859,7 @@ static void destroy_call_elem(grpc_call_element* elem,
   call_data* calld = static_cast<call_data*>(elem->call_data);
 
   GPR_ASSERT(calld->state != PENDING);
-
+  GRPC_ERROR_UNREF(calld->error);
   if (calld->host_set) {
     grpc_slice_unref_internal(calld->host);
   }