fix: Modify ConvertExceptionCallable to retry on Goaway (#1588)

jackdingilian · web-flow · commit cf752eaf3b5f · 2023-02-07T20:04:13.000Z
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConvertExceptionCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConvertExceptionCallable.java
@@ -21,6 +21,7 @@
 import com.google.api.gax.rpc.ResponseObserver;
 import com.google.api.gax.rpc.ServerStreamingCallable;
 import com.google.api.gax.rpc.StreamController;
+import com.google.common.base.Throwables;
 
 /**
  * This callable converts the "Received rst stream" exception into a retryable {@link ApiException}.
@@ -73,14 +74,29 @@ protected void onCompleteImpl() {
   }
 
   private Throwable convertException(Throwable t) {
-    // Long lived connections sometimes are disconnected via an RST frame. This error is
-    // transient and should be retried.
+    // Long lived connections sometimes are disconnected via an RST frame or a goaway. These errors
+    // are transient and should be retried.
+    if (isRstStreamError(t) || isGoAway(t)) {
+      return new InternalException(t, ((InternalException) t).getStatusCode(), true);
+    }
+    return t;
+  }
+
+  private boolean isRstStreamError(Throwable t) {
     if (t instanceof InternalException && t.getMessage() != null) {
       String error = t.getMessage().toLowerCase();
-      if (error.contains("rst_stream") || error.contains("rst stream")) {
-        return new InternalException(t, ((InternalException) t).getStatusCode(), true);
-      }
+      return error.contains("rst_stream") || error.contains("rst stream");
     }
-    return t;
+    return false;
+  }
+
+  private boolean isGoAway(Throwable t) {
+    if (t instanceof InternalException) {
+      Throwable rootCause = Throwables.getRootCause(t);
+      String rootCauseMessage = rootCause.getMessage();
+      return rootCauseMessage != null
+          && rootCauseMessage.contains("Stream closed before write could take place");
+    }
+    return false;
   }
 }
