Error handling progress in chttp2 transport

6 files changed, 127 insertions, 118 deletions

diff --git a/src/core/ext/transport/chttp2/transport/frame_rst_stream.c b/src/core/ext/transport/chttp2/transport/frame_rst_stream.c
index d49df0fcc2..ea71123054 100644
--- a/src/core/ext/transport/chttp2/transport/frame_rst_stream.c
+++ b/src/core/ext/transport/chttp2/transport/frame_rst_stream.c
@@ -97,11 +97,12 @@ grpc_error *grpc_chttp2_rst_stream_parser_parse(
   if (p->byte == 4) {
     GPR_ASSERT(is_last);
     stream_parsing->received_close = 1;
-    stream_parsing->saw_rst_stream = 1;
-    stream_parsing->rst_stream_reason = (((uint32_t)p->reason_bytes[0]) << 24) |
-                                        (((uint32_t)p->reason_bytes[1]) << 16) |
-                                        (((uint32_t)p->reason_bytes[2]) << 8) |
-                                        (((uint32_t)p->reason_bytes[3]));
+    stream_parsing->forced_close_error = grpc_error_set_int(
+        GRPC_ERROR_CREATE("RST_STREAM"), GRPC_ERROR_INT_HTTP2_ERROR,
+        (((uint32_t)p->reason_bytes[0]) << 24) |
+            (((uint32_t)p->reason_bytes[1]) << 16) |
+            (((uint32_t)p->reason_bytes[2]) << 8) |
+            (((uint32_t)p->reason_bytes[3])));
   }
 
   return GRPC_ERROR_NONE;
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index ae35d58117..1a8b44d86e 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -474,13 +474,13 @@ struct grpc_chttp2_stream_parsing {
   uint32_t id;
   /** has this stream received a close */
   uint8_t received_close;
-  /** saw a rst_stream */
-  uint8_t saw_rst_stream;
   /** how many header frames have we received? */
   uint8_t header_frames_received;
   /** which metadata did we get (on this parse) */
   uint8_t got_metadata_on_parse[2];
-  /** should we raise the seen_error flag in transport_global */
+  /** saw some stream level error */
+  grpc_error *forced_close_error;
+  /** should we raise the seen_error flag in stream_global */
   uint8_t seen_error;
   /** window available for peer to send to us */
   int64_t incoming_window;
diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c
index aaa66a6c91..67590aeb16 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.c
+++ b/src/core/ext/transport/chttp2/transport/parsing.c
@@ -83,8 +83,8 @@ void grpc_chttp2_prepare_to_read(
          transport_global->settings[GRPC_SENT_SETTINGS],
          sizeof(transport_parsing->last_sent_settings));
   transport_parsing->max_frame_size =
-      transport_global
-          ->settings[GRPC_ACKED_SETTINGS][GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE];
+      transport_global->settings[GRPC_ACKED_SETTINGS]
+                                [GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE];
 
   /* update the parsing view of incoming window */
   while (grpc_chttp2_list_pop_unannounced_incoming_window_available(
@@ -224,26 +224,29 @@ void grpc_chttp2_publish_reads(
       grpc_chttp2_list_add_check_read_ops(transport_global, stream_global);
     }
 
-    if (stream_parsing->saw_rst_stream) {
-      if (stream_parsing->rst_stream_reason != GRPC_CHTTP2_NO_ERROR) {
-        grpc_status_code status_code = grpc_chttp2_http2_error_to_grpc_status(
-            (grpc_chttp2_error_code)stream_parsing->rst_stream_reason);
-        char *status_details;
-        gpr_slice slice_details;
-        gpr_asprintf(&status_details, "Received RST_STREAM err=%d",
-                     stream_parsing->rst_stream_reason);
-        slice_details = gpr_slice_from_copied_string(status_details);
-        gpr_free(status_details);
+    if (stream_parsing->forced_close_error != GRPC_ERROR_NONE) {
+      const intptr_t *reason = grpc_error_get_int(
+          stream_parsing->forced_close_error, GRPC_ERROR_INT_HTTP2_ERROR);
+      if (reason == NULL || *reason != GRPC_CHTTP2_NO_ERROR) {
+        grpc_status_code status_code =
+            reason == NULL ? GRPC_STATUS_INTERNAL
+                           : grpc_chttp2_http2_error_to_grpc_status(
+                                 (grpc_chttp2_error_code)
+                                     stream_parsing->rst_stream_reason);
+        const char *status_details =
+            grpc_error_string(stream_parsing->forced_close_error);
+        gpr_slice slice_details = gpr_slice_from_copied_string(status_details);
+        grpc_error_free_string(status_details);
         grpc_chttp2_fake_status(exec_ctx, transport_global, stream_global,
                                 status_code, &slice_details);
       }
       grpc_chttp2_mark_stream_closed(exec_ctx, transport_global, stream_global,
-                                     1, 1);
+                                     1, 1, stream_parsing->forced_close_error);
     }
 
     if (stream_parsing->received_close) {
       grpc_chttp2_mark_stream_closed(exec_ctx, transport_global, stream_global,
-                                     1, 0);
+                                     1, 0, GRPC_ERROR_NONE);
     }
   }
 }
@@ -577,7 +580,7 @@ static grpc_error *update_incoming_window(
 
   grpc_chttp2_list_add_parsing_seen_stream(transport_parsing, stream_parsing);
 
-  return GRPC_CHTTP2_PARSE_OK;
+  return GRPC_ERROR_NONE;
 }
 
 static grpc_error *init_data_frame_parser(
@@ -585,7 +588,7 @@ static grpc_error *init_data_frame_parser(
   grpc_chttp2_stream_parsing *stream_parsing =
       grpc_chttp2_parsing_lookup_stream(transport_parsing,
                                         transport_parsing->incoming_stream_id);
-  grpc_chttp2_parse_error err = GRPC_CHTTP2_PARSE_OK;
+  grpc_error *err = GRPC_ERROR_NONE;
   if (stream_parsing == NULL) {
     return init_skip_frame_parser(exec_ctx, transport_parsing, 0);
   }
@@ -593,33 +596,32 @@ static grpc_error *init_data_frame_parser(
   if (stream_parsing->received_close) {
     return init_skip_frame_parser(exec_ctx, transport_parsing, 0);
   }
-  if (err == GRPC_CHTTP2_PARSE_OK) {
+  if (err == GRPC_ERROR_NONE) {
     err = update_incoming_window(exec_ctx, transport_parsing, stream_parsing);
   }
-  if (err == GRPC_CHTTP2_PARSE_OK) {
+  if (err == GRPC_ERROR_NONE) {
     err = grpc_chttp2_data_parser_begin_frame(
-        &stream_parsing->data_parser, transport_parsing->incoming_frame_flags);
-  }
-  switch (err) {
-    case GRPC_CHTTP2_PARSE_OK:
-      transport_parsing->incoming_stream = stream_parsing;
-      transport_parsing->parser = grpc_chttp2_data_parser_parse;
-      transport_parsing->parser_data = &stream_parsing->data_parser;
-      return GRPC_ERROR_NONE;
-    case GRPC_CHTTP2_STREAM_ERROR:
-      stream_parsing->received_close = 1;
-      stream_parsing->saw_rst_stream = 1;
-      stream_parsing->rst_stream_reason = GRPC_CHTTP2_PROTOCOL_ERROR;
-      gpr_slice_buffer_add(
-          &transport_parsing->qbuf,
-          grpc_chttp2_rst_stream_create(transport_parsing->incoming_stream_id,
-                                        GRPC_CHTTP2_PROTOCOL_ERROR,
-                                        &stream_parsing->stats.outgoing));
-      return init_skip_frame_parser(exec_ctx, transport_parsing, 0);
-    case GRPC_CHTTP2_CONNECTION_ERROR:
-      return 0;
+        &stream_parsing->data_parser, transport_parsing->incoming_frame_flags,
+        stream_parsing->id);
+  }
+  if (err == GRPC_ERROR_NONE) {
+    transport_parsing->incoming_stream = stream_parsing;
+    transport_parsing->parser = grpc_chttp2_data_parser_parse;
+    transport_parsing->parser_data = &stream_parsing->data_parser;
+    return GRPC_ERROR_NONE;
+  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID)) {
+    /* handle stream errors by closing the stream */
+    stream_parsing->received_close = 1;
+    stream_parsing->forced_close_error = err;
+    gpr_slice_buffer_add(
+        &transport_parsing->qbuf,
+        grpc_chttp2_rst_stream_create(transport_parsing->incoming_stream_id,
+                                      GRPC_CHTTP2_PROTOCOL_ERROR,
+                                      &stream_parsing->stats.outgoing));
+    return init_skip_frame_parser(exec_ctx, transport_parsing, 0);
+  } else {
+    return err;
   }
-  GPR_UNREACHABLE_CODE(return 0);
 }
 
 static void free_timeout(void *p) { gpr_free(p); }
@@ -797,10 +799,11 @@ static grpc_error *init_header_frame_parser(
 
 static grpc_error *init_window_update_frame_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
-  int ok = GRPC_CHTTP2_PARSE_OK == grpc_chttp2_window_update_parser_begin_frame(
-                                       &transport_parsing->simple.window_update,
-                                       transport_parsing->incoming_frame_size,
-                                       transport_parsing->incoming_frame_flags);
+  grpc_error *err = grpc_chttp2_window_update_parser_begin_frame(
+      &transport_parsing->simple.window_update,
+      transport_parsing->incoming_frame_size,
+      transport_parsing->incoming_frame_flags);
+  if (err != GRPC_ERROR_NONE) return err;
   if (transport_parsing->incoming_stream_id != 0) {
     grpc_chttp2_stream_parsing *stream_parsing =
         transport_parsing->incoming_stream = grpc_chttp2_parsing_lookup_stream(
@@ -812,26 +815,27 @@ static grpc_error *init_window_update_frame_parser(
   }
   transport_parsing->parser = grpc_chttp2_window_update_parser_parse;
   transport_parsing->parser_data = &transport_parsing->simple.window_update;
-  return ok;
+  return GRPC_ERROR_NONE;
 }
 
-static int init_ping_parser(grpc_exec_ctx *exec_ctx,
-                            grpc_chttp2_transport_parsing *transport_parsing) {
-  int ok = GRPC_CHTTP2_PARSE_OK == grpc_chttp2_ping_parser_begin_frame(
-                                       &transport_parsing->simple.ping,
-                                       transport_parsing->incoming_frame_size,
-                                       transport_parsing->incoming_frame_flags);
+static grpc_error *init_ping_parser(
+    grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
+  grpc_error *err = grpc_chttp2_ping_parser_begin_frame(
+      &transport_parsing->simple.ping, transport_parsing->incoming_frame_size,
+      transport_parsing->incoming_frame_flags);
+  if (err != GRPC_ERROR_NONE) return err;
   transport_parsing->parser = grpc_chttp2_ping_parser_parse;
   transport_parsing->parser_data = &transport_parsing->simple.ping;
-  return ok;
+  return GRPC_ERROR_NONE;
 }
 
-static int init_rst_stream_parser(
+static grpc_error *init_rst_stream_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
-  int ok = GRPC_CHTTP2_PARSE_OK == grpc_chttp2_rst_stream_parser_begin_frame(
-                                       &transport_parsing->simple.rst_stream,
-                                       transport_parsing->incoming_frame_size,
-                                       transport_parsing->incoming_frame_flags);
+  grpc_error *err = grpc_chttp2_rst_stream_parser_begin_frame(
+      &transport_parsing->simple.rst_stream,
+      transport_parsing->incoming_frame_size,
+      transport_parsing->incoming_frame_flags);
+  if (err != GRPC_ERROR_NONE) return err;
   grpc_chttp2_stream_parsing *stream_parsing =
       transport_parsing->incoming_stream = grpc_chttp2_parsing_lookup_stream(
           transport_parsing, transport_parsing->incoming_stream_id);
@@ -841,37 +845,32 @@ static int init_rst_stream_parser(
   stream_parsing->stats.incoming.framing_bytes += 9;
   transport_parsing->parser = grpc_chttp2_rst_stream_parser_parse;
   transport_parsing->parser_data = &transport_parsing->simple.rst_stream;
-  return ok;
+  return GRPC_ERROR_NONE;
 }
 
-static int init_goaway_parser(
+static grpc_error *init_goaway_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
-  int ok = GRPC_CHTTP2_PARSE_OK == grpc_chttp2_goaway_parser_begin_frame(
-                                       &transport_parsing->goaway_parser,
-                                       transport_parsing->incoming_frame_size,
-                                       transport_parsing->incoming_frame_flags);
+  grpc_error *err = grpc_chttp2_goaway_parser_begin_frame(
+      &transport_parsing->goaway_parser, transport_parsing->incoming_frame_size,
+      transport_parsing->incoming_frame_flags);
+  if (err != GRPC_ERROR_NONE) return err;
   transport_parsing->parser = grpc_chttp2_goaway_parser_parse;
   transport_parsing->parser_data = &transport_parsing->goaway_parser;
-  return ok;
+  return GRPC_ERROR_NONE;
 }
 
-static int init_settings_frame_parser(
+static grpc_error *init_settings_frame_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
-  int ok;
-
   if (transport_parsing->incoming_stream_id != 0) {
-    gpr_log(GPR_ERROR, "settings frame received for grpc_chttp2_stream %d",
-            transport_parsing->incoming_stream_id);
-    return 0;
+    return GRPC_ERROR_CREATE("Settings frame received for grpc_chttp2_stream");
   }
 
-  ok = GRPC_CHTTP2_PARSE_OK == grpc_chttp2_settings_parser_begin_frame(
-                                   &transport_parsing->simple.settings,
-                                   transport_parsing->incoming_frame_size,
-                                   transport_parsing->incoming_frame_flags,
-                                   transport_parsing->settings);
-  if (!ok) {
-    return 0;
+  grpc_error *err = grpc_chttp2_settings_parser_begin_frame(
+      &transport_parsing->simple.settings,
+      transport_parsing->incoming_frame_size,
+      transport_parsing->incoming_frame_flags, transport_parsing->settings);
+  if (err != GRPC_ERROR_NONE) {
+    return err;
   }
   if (transport_parsing->incoming_frame_flags & GRPC_CHTTP2_FLAG_ACK) {
     transport_parsing->settings_ack_received = 1;
@@ -885,7 +884,7 @@ static int init_settings_frame_parser(
   }
   transport_parsing->parser = grpc_chttp2_settings_parser_parse;
   transport_parsing->parser_data = &transport_parsing->simple.settings;
-  return ok;
+  return GRPC_ERROR_NONE;
 }
 
 /*
@@ -894,34 +893,37 @@ static int is_window_update_legal(int64_t window_update, int64_t window) {
 }
 */
 
-static int parse_frame_slice(grpc_exec_ctx *exec_ctx,
-                             grpc_chttp2_transport_parsing *transport_parsing,
-                             gpr_slice slice, int is_last) {
+static grpc_error *parse_frame_slice(
+    grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing,
+    gpr_slice slice, int is_last) {
   grpc_chttp2_stream_parsing *stream_parsing =
       transport_parsing->incoming_stream;
-  switch (transport_parsing->parser(exec_ctx, transport_parsing->parser_data,
-                                    transport_parsing, stream_parsing, slice,
-                                    is_last)) {
-    case GRPC_CHTTP2_PARSE_OK:
-      if (stream_parsing) {
-        grpc_chttp2_list_add_parsing_seen_stream(transport_parsing,
-                                                 stream_parsing);
-      }
-      return 1;
-    case GRPC_CHTTP2_STREAM_ERROR:
-      grpc_chttp2_parsing_become_skip_parser(exec_ctx, transport_parsing);
-      if (stream_parsing) {
-        stream_parsing->saw_rst_stream = 1;
-        stream_parsing->rst_stream_reason = GRPC_CHTTP2_PROTOCOL_ERROR;
-        gpr_slice_buffer_add(
-            &transport_parsing->qbuf,
-            grpc_chttp2_rst_stream_create(transport_parsing->incoming_stream_id,
-                                          GRPC_CHTTP2_PROTOCOL_ERROR,
-                                          &stream_parsing->stats.outgoing));
-      }
-      return 1;
-    case GRPC_CHTTP2_CONNECTION_ERROR:
-      return 0;
+  grpc_error *err = transport_parsing->parser(
+      exec_ctx, transport_parsing->parser_data, transport_parsing,
+      stream_parsing, slice, is_last);
+  if (err == GRPC_ERROR_NONE) {
+    if (stream_parsing) {
+      grpc_chttp2_list_add_parsing_seen_stream(transport_parsing,
+                                               stream_parsing);
+    }
+    return GRPC_ERROR_NONE;
+  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID) != NULL) {
+    if (grpc_http_trace) {
+      const char *msg = grpc_error_string(err);
+      gpr_log(GPR_ERROR, "%s", msg);
+      grpc_error_free_string(msg);
+    }
+    grpc_chttp2_parsing_become_skip_parser(exec_ctx, transport_parsing);
+    if (stream_parsing) {
+      stream_parsing->forced_close_error = err;
+      gpr_slice_buffer_add(
+          &transport_parsing->qbuf,
+          grpc_chttp2_rst_stream_create(transport_parsing->incoming_stream_id,
+                                        GRPC_CHTTP2_PROTOCOL_ERROR,
+                                        &stream_parsing->stats.outgoing));
+    } else {
+      grpc_error_unref(err);
+    }
   }
-  GPR_UNREACHABLE_CODE(return 0);
+  return err;
 }
diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c
index a8fb463939..ff42fd249a 100644
--- a/src/core/ext/transport/chttp2/transport/writing.c
+++ b/src/core/ext/transport/chttp2/transport/writing.c
@@ -187,7 +187,8 @@ void grpc_chttp2_perform_writes(
     grpc_endpoint_write(exec_ctx, endpoint, &transport_writing->outbuf,
                         &transport_writing->done_cb);
   } else {
-    grpc_exec_ctx_enqueue(exec_ctx, &transport_writing->done_cb, true, NULL);
+    grpc_exec_ctx_push(exec_ctx, &transport_writing->done_cb, GRPC_ERROR_NONE,
+                       NULL);
   }
 }
 
@@ -335,24 +336,26 @@ void grpc_chttp2_cleanup_writing(
     if (stream_writing->sent_initial_metadata) {
       grpc_chttp2_complete_closure_step(
           exec_ctx, stream_global,
-          &stream_global->send_initial_metadata_finished, 1);
+          &stream_global->send_initial_metadata_finished, GRPC_ERROR_NONE);
     }
     grpc_transport_move_one_way_stats(&stream_writing->stats,
                                       &stream_global->stats.outgoing);
     if (stream_writing->sent_message) {
       GPR_ASSERT(stream_writing->send_message == NULL);
-      grpc_chttp2_complete_closure_step(
-          exec_ctx, stream_global, &stream_global->send_message_finished, 1);
+      grpc_chttp2_complete_closure_step(exec_ctx, stream_global,
+                                        &stream_global->send_message_finished,
+                                        GRPC_ERROR_NONE);
       stream_writing->sent_message = 0;
     }
     if (stream_writing->sent_trailing_metadata) {
       grpc_chttp2_complete_closure_step(
           exec_ctx, stream_global,
-          &stream_global->send_trailing_metadata_finished, 1);
+          &stream_global->send_trailing_metadata_finished, GRPC_ERROR_NONE);
     }
     if (stream_writing->sent_trailing_metadata) {
       grpc_chttp2_mark_stream_closed(exec_ctx, transport_global, stream_global,
-                                     !transport_global->is_client, 1);
+                                     !transport_global->is_client, 1,
+                                     GRPC_ERROR_NONE);
     }
     GRPC_CHTTP2_STREAM_UNREF(exec_ctx, stream_global, "chttp2_writing");
   }
diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c
index 55bb1b5660..5812af5c93 100644
--- a/src/core/lib/iomgr/error.c
+++ b/src/core/lib/iomgr/error.c
@@ -102,6 +102,8 @@ static const char *error_int_name(grpc_error_ints key) {
       return "index";
     case GRPC_ERROR_INT_SIZE:
       return "size";
+    case GRPC_ERROR_INT_HTTP2_ERROR:
+      return "http2_error";
   }
   GPR_UNREACHABLE_CODE(return "unknown");
 }
diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h
index db5e378e77..234782ab03 100644
--- a/src/core/lib/iomgr/error.h
+++ b/src/core/lib/iomgr/error.h
@@ -50,6 +50,7 @@ typedef enum {
   GRPC_ERROR_INT_OFFSET,
   GRPC_ERROR_INT_INDEX,
   GRPC_ERROR_INT_SIZE,
+  GRPC_ERROR_INT_HTTP2_ERROR,
 } grpc_error_ints;
 
 typedef enum {