fix: update grpc resumable upload error categorization to be more tol…

…erant (#2644) When a client is shutting down, the Channel pool will deliver an error to our stream observer. This could happen before we are able to actually write a message. Update GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel to not attempt to pass null to ImmutableList.of().
googleapis · Jul 24, 2024 · 95697dd · 95697dd
1 parent 1be23c9
commit 95697dd
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 7 deletions.
diff --git a/.../com/google/cloud/storage/GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel.java b/.../com/google/cloud/storage/GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel.java
@@ -17,6 +17,7 @@
 package com.google.cloud.storage;
 
 import static com.google.cloud.storage.GrpcUtils.contextWithBucketName;
+import static com.google.cloud.storage.Utils.nullSafeList;
 
 import com.google.api.core.SettableApiFuture;
 import com.google.api.gax.grpc.GrpcCallContext;
@@ -26,7 +27,6 @@
 import com.google.cloud.storage.ChunkSegmenter.ChunkSegment;
 import com.google.cloud.storage.Crc32cValue.Crc32cLengthKnown;
 import com.google.cloud.storage.UnbufferedWritableByteChannelSession.UnbufferedWritableByteChannel;
-import com.google.common.collect.ImmutableList;
 import com.google.protobuf.ByteString;
 import com.google.storage.v2.ChecksummedData;
 import com.google.storage.v2.ObjectChecksums;
@@ -230,7 +230,7 @@ public void onError(Throwable t) {
                 tmp.getCode(),
                 tmp.getMessage(),
                 tmp.getReason(),
-                ImmutableList.of(lastWrittenRequest),
+                nullSafeList(lastWrittenRequest),
                 null,
                 context,
                 t);
@@ -251,7 +251,7 @@ public void onCompleted() {
                 0,
                 "onComplete without preceding onNext, unable to determine success.",
                 "invalid",
-                ImmutableList.of(lastWrittenRequest),
+                nullSafeList(lastWrittenRequest),
                 null,
                 context,
                 null));
@@ -263,26 +263,26 @@ public void onCompleted() {
         } else if (finalSize < totalSentBytes) {
           clientDetectedError(
               ResumableSessionFailureScenario.SCENARIO_4_1.toStorageException(
-                  ImmutableList.of(lastWrittenRequest), last, context, null));
+                  nullSafeList(lastWrittenRequest), last, context, null));
         } else {
           clientDetectedError(
               ResumableSessionFailureScenario.SCENARIO_4_2.toStorageException(
-                  ImmutableList.of(lastWrittenRequest), last, context, null));
+                  nullSafeList(lastWrittenRequest), last, context, null));
         }
       } else if (!finalizing || last.hasPersistedSize()) { // unexpected incremental response
         clientDetectedError(
             ResumableSessionFailureScenario.toStorageException(
                 0,
                 "Unexpected incremental response for finalizing request.",
                 "invalid",
-                ImmutableList.of(lastWrittenRequest),
+                nullSafeList(lastWrittenRequest),
                 last,
                 context,
                 null));
       } else {
         clientDetectedError(
             ResumableSessionFailureScenario.SCENARIO_0.toStorageException(
-                ImmutableList.of(lastWrittenRequest), last, context, null));
+                nullSafeList(lastWrittenRequest), last, context, null));
       }
     }
 

diff --git a/...cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java b/...cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java
@@ -169,6 +169,9 @@ static StorageException toStorageException(
     Map<String, List<String>> extraHeaders = context.getExtraHeaders();
     recordHeadersTo(extraHeaders, PREFIX_O, sb);
     int length = reqs.size();
+    if (length == 0) {
+      sb.append("\n").append(PREFIX_O).append("[]");
+    }
     for (int i = 0; i < length; i++) {
       if (i == 0) {
         sb.append("\n").append(PREFIX_O).append("[");

diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java
@@ -25,6 +25,7 @@
 import com.google.cloud.storage.Conversions.Codec;
 import com.google.cloud.storage.UnifiedOpts.NamedField;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.MapDifference;
 import com.google.common.collect.Maps;
 import com.google.common.io.BaseEncoding;
@@ -310,4 +311,12 @@ private static String crc32cEncode(int from) {
   static GrpcCallContext merge(@NonNull GrpcCallContext l, @NonNull GrpcCallContext r) {
     return (GrpcCallContext) l.merge(r);
   }
+
+  static <T> ImmutableList<T> nullSafeList(@Nullable T t) {
+    if (t == null) {
+      return ImmutableList.of();
+    } else {
+      return ImmutableList.of(t);
+    }
+  }
 }