Merge pull request #7291 from ctiller/better_status

Make it more likely to correctly report deadline exceeded

6 files changed, 30 insertions, 12 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 38e782b9b4..5aae753c07 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -513,6 +513,7 @@ static int init_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt,
       &s->global.received_trailing_metadata);
   grpc_chttp2_data_parser_init(&s->parsing.data_parser);
   gpr_slice_buffer_init(&s->writing.flow_controlled_buffer);
+  s->global.deadline = gpr_inf_future(GPR_CLOCK_MONOTONIC);
 
   REF_TRANSPORT(t, "stream");
 
@@ -988,6 +989,11 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx,
     const size_t metadata_peer_limit =
         transport_global->settings[GRPC_PEER_SETTINGS]
                                   [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
+    if (transport_global->is_client) {
+      stream_global->deadline =
+          gpr_time_min(stream_global->deadline,
+                       stream_global->send_initial_metadata->deadline);
+    }
     if (metadata_size > metadata_peer_limit) {
       cancel_from_api(
           exec_ctx, transport_global, stream_global,
@@ -1366,7 +1372,7 @@ static void remove_stream(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
   GRPC_ERROR_UNREF(error);
 }
 
-static void status_codes_from_error(grpc_error *error,
+static void status_codes_from_error(grpc_error *error, gpr_timespec deadline,
                                     grpc_chttp2_error_code *http2_error,
                                     grpc_status_code *grpc_status) {
   intptr_t ip_http;
@@ -1386,8 +1392,8 @@ static void status_codes_from_error(grpc_error *error,
   if (have_grpc) {
     *grpc_status = (grpc_status_code)ip_grpc;
   } else if (have_http) {
-    *grpc_status =
-        grpc_chttp2_http2_error_to_grpc_status((grpc_chttp2_error_code)ip_http);
+    *grpc_status = grpc_chttp2_http2_error_to_grpc_status(
+        (grpc_chttp2_error_code)ip_http, deadline);
   } else {
     *grpc_status = GRPC_STATUS_INTERNAL;
   }
@@ -1400,7 +1406,8 @@ static void cancel_from_api(grpc_exec_ctx *exec_ctx,
   if (!stream_global->read_closed || !stream_global->write_closed) {
     grpc_status_code grpc_status;
     grpc_chttp2_error_code http_error;
-    status_codes_from_error(due_to_error, &http_error, &grpc_status);
+    status_codes_from_error(due_to_error, stream_global->deadline, &http_error,
+                            &grpc_status);
 
     if (stream_global->id != 0) {
       gpr_slice_buffer_add(
@@ -1536,7 +1543,8 @@ static void close_from_api(grpc_exec_ctx *exec_ctx,
   uint32_t len = 0;
   grpc_status_code grpc_status;
   grpc_chttp2_error_code http_error;
-  status_codes_from_error(error, &http_error, &grpc_status);
+  status_codes_from_error(error, stream_global->deadline, &http_error,
+                          &grpc_status);
 
   GPR_ASSERT(grpc_status >= 0 && (int)grpc_status < 100);
 
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index b5180c6fc8..8d79e93ceb 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -447,6 +447,8 @@ typedef struct {
   grpc_chttp2_incoming_metadata_buffer received_trailing_metadata;
 
   grpc_chttp2_incoming_frame_queue incoming_frames;
+
+  gpr_timespec deadline;
 } grpc_chttp2_stream_global;
 
 typedef struct {
diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c
index 991d7729af..84eb5752f1 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.c
+++ b/src/core/ext/transport/chttp2/transport/parsing.c
@@ -236,9 +236,10 @@ void grpc_chttp2_publish_reads(
                                            GRPC_ERROR_INT_HTTP2_ERROR, &reason);
       if (has_reason && reason != GRPC_CHTTP2_NO_ERROR) {
         grpc_status_code status_code =
-            has_reason ? grpc_chttp2_http2_error_to_grpc_status(
-                             (grpc_chttp2_error_code)reason)
-                       : GRPC_STATUS_INTERNAL;
+            has_reason
+                ? grpc_chttp2_http2_error_to_grpc_status(
+                      (grpc_chttp2_error_code)reason, stream_global->deadline)
+                : GRPC_STATUS_INTERNAL;
         const char *status_details =
             grpc_error_string(stream_parsing->forced_close_error);
         gpr_slice slice_details = gpr_slice_from_copied_string(status_details);
diff --git a/src/core/ext/transport/chttp2/transport/status_conversion.c b/src/core/ext/transport/chttp2/transport/status_conversion.c
index c42fb9b3a1..5dce2f2d0c 100644
--- a/src/core/ext/transport/chttp2/transport/status_conversion.c
+++ b/src/core/ext/transport/chttp2/transport/status_conversion.c
@@ -39,6 +39,8 @@ int grpc_chttp2_grpc_status_to_http2_error(grpc_status_code status) {
       return GRPC_CHTTP2_NO_ERROR;
     case GRPC_STATUS_CANCELLED:
       return GRPC_CHTTP2_CANCEL;
+    case GRPC_STATUS_DEADLINE_EXCEEDED:
+      return GRPC_CHTTP2_CANCEL;
     case GRPC_STATUS_RESOURCE_EXHAUSTED:
       return GRPC_CHTTP2_ENHANCE_YOUR_CALM;
     case GRPC_STATUS_PERMISSION_DENIED:
@@ -51,13 +53,17 @@ int grpc_chttp2_grpc_status_to_http2_error(grpc_status_code status) {
 }
 
 grpc_status_code grpc_chttp2_http2_error_to_grpc_status(
-    grpc_chttp2_error_code error) {
+    grpc_chttp2_error_code error, gpr_timespec deadline) {
   switch (error) {
     case GRPC_CHTTP2_NO_ERROR:
       /* should never be received */
       return GRPC_STATUS_INTERNAL;
     case GRPC_CHTTP2_CANCEL:
-      return GRPC_STATUS_CANCELLED;
+      /* http2 cancel translates to STATUS_CANCELLED iff deadline hasn't been
+       * exceeded */
+      return gpr_time_cmp(gpr_now(deadline.clock_type), deadline) >= 0
+                 ? GRPC_STATUS_DEADLINE_EXCEEDED
+                 : GRPC_STATUS_CANCELLED;
     case GRPC_CHTTP2_ENHANCE_YOUR_CALM:
       return GRPC_STATUS_RESOURCE_EXHAUSTED;
     case GRPC_CHTTP2_INADEQUATE_SECURITY:
diff --git a/src/core/ext/transport/chttp2/transport/status_conversion.h b/src/core/ext/transport/chttp2/transport/status_conversion.h
index e7285e6fd5..953bc9f1e1 100644
--- a/src/core/ext/transport/chttp2/transport/status_conversion.h
+++ b/src/core/ext/transport/chttp2/transport/status_conversion.h
@@ -41,7 +41,7 @@
 grpc_chttp2_error_code grpc_chttp2_grpc_status_to_http2_error(
     grpc_status_code status);
 grpc_status_code grpc_chttp2_http2_error_to_grpc_status(
-    grpc_chttp2_error_code error);
+    grpc_chttp2_error_code error, gpr_timespec deadline);
 
 /* Conversion of HTTP status codes (:status) to grpc status codes */
 grpc_status_code grpc_chttp2_http2_status_to_grpc_status(int status);
diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c
index e5668be47f..fc9df76dc1 100644
--- a/src/core/lib/surface/call.c
+++ b/src/core/lib/surface/call.c
@@ -259,7 +259,8 @@ grpc_call *grpc_call_create(
       call->metadata_batch[i][j].deadline = gpr_inf_future(GPR_CLOCK_MONOTONIC);
     }
   }
-  call->send_deadline = send_deadline;
+  call->send_deadline =
+      gpr_convert_clock_type(send_deadline, GPR_CLOCK_MONOTONIC);
   GRPC_CHANNEL_INTERNAL_REF(channel, "call");
   /* initial refcount dropped by grpc_call_destroy */
   grpc_call_stack_init(&exec_ctx, channel_stack, 1, destroy_call, call,