From 95697dd3d744351058c13793c6ae576820f6b638 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Wed, 24 Jul 2024 13:03:16 -0400 Subject: [PATCH] fix: update grpc resumable upload error categorization to be more tolerant (#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(). --- ...inalizeOnCloseResumableWritableByteChannel.java | 14 +++++++------- .../storage/ResumableSessionFailureScenario.java | 3 +++ .../main/java/com/google/cloud/storage/Utils.java | 9 +++++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel.java index 984c7bfd8f..e85363b227 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicUnbufferedFinalizeOnCloseResumableWritableByteChannel.java +++ b/google-cloud-storage/src/main/java/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,11 +263,11 @@ 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( @@ -275,14 +275,14 @@ public void onCompleted() { 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/google-cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java index 88dbaf7bdc..aab5263397 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/ResumableSessionFailureScenario.java @@ -169,6 +169,9 @@ static StorageException toStorageException( Map> 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 index c24a68d4d6..c0374d3f06 100644 --- 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 ImmutableList nullSafeList(@Nullable T t) { + if (t == null) { + return ImmutableList.of(); + } else { + return ImmutableList.of(t); + } + } }