fix retry in GCS storage client

Author: sfc-gh-dszmolka

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 # Changelog
 - v4.3.2-SNAPSHOT
+  - Fixed GCS PUT operations not retrying on transient errors (e.g. HTTP 503) despite `putgetmaxretries` being configured (snowflakedb/snowflake-jdbc#2688).
   - Bumped jackson-databind to 2.18.8 from 2.18.7 (snowflakedb/snowflake-jdbc#2669).
   - Fixed snowflake-jdbc writing a `snowflake-minicore-*` temp directory and loading the native library at driver class-load time even when the driver was never used (e.g. when present on the classpath only as a transitive dependency). Minicore now loads lazily when the first Snowflake connection is created (`ConnectionFactory.createConnection`) instead of during `DriverInitializer.initialize()` (snowflakedb/snowflake-jdbc#2670).
   - Restored `GetCallerIdentity` as the default AWS Workload Identity Federation attestation method to avoid breaking existing users who have not configured the `ISSUER` in their Snowflake WIF setup. The `GetWebIdentityToken` (outbound JWT) flow introduced in v4.3.0 is now opt-in via the new `workloadIdentityAwsUseOutboundToken` connection property.

src/main/java/net/snowflake/client/internal/jdbc/cloud/storage/GCSAccessStrategyAwsSdk.java

@@ -261,46 +261,20 @@ public boolean handleStorageException(
       String queryId,
       SnowflakeGCSClient gcsClient)
       throws SnowflakeSQLException {
-    // Find an SdkException either directly in the cause or anywhere in the cause chain.
-    // When the exception originates from the inner uploadWithDownScopedToken it arrives wrapped
-    // in SnowflakeSQLLoggedException(CompletionException(SdkException)); walking the chain
-    // ensures transient errors are always treated as retryable regardless of wrapping depth.
-    SdkException sdkEx = null;
     Throwable cause = ex.getCause();
-    while (cause != null) {
-      if (cause instanceof SdkException) {
-        sdkEx = (SdkException) cause;
-        break;
-      }
-      cause = cause.getCause();
-    }
-
-    if (sdkEx != null) {
-      if (sdkEx != ex.getCause()) {
-        logger.debug(
-            "GCSAccessStrategyAwsSdk: found SdkException wrapped inside {} during {};"
-                + " treating as retryable",
-            ex.getClass().getSimpleName(),
-            operation);
-      }
-      logger.debug("GCSAccessStrategyAwsSdk: " + sdkEx.getMessage());
+    if (cause instanceof SdkException) {
+      logger.debug("GCSAccessStrategyAwsSdk: " + cause.getMessage());
 
       if (retryCount > gcsClient.getMaxRetries()
-          || S3ErrorHandler.isClientException400Or404(sdkEx)) {
+          || S3ErrorHandler.isClientException400Or404(cause)) {
         throwIfClientExceptionOrMaxRetryReached(
-            operation, session, command, queryId, gcsClient, sdkEx);
+            operation, session, command, queryId, gcsClient, cause);
       } else {
         retryRequestWithExponentialBackoff(
-            ex, retryCount, operation, session, command, gcsClient, queryId, sdkEx);
+            ex, retryCount, operation, session, command, gcsClient, queryId, cause);
       }
       return true;
     } else {
-      logger.error(
-          "GCSAccessStrategyAwsSdk: unhandled exception type {} during {}, not retrying: {}",
-          ex.getClass().getName(),
-          operation,
-          ex.getMessage());
-      logger.error("Stack trace: ", ex);
       return false;
     }
   }

src/main/java/net/snowflake/client/internal/jdbc/cloud/storage/GCSDefaultAccessStrategy.java

@@ -207,7 +207,8 @@ public boolean handleStorageException(
     // The exception may arrive wrapped (e.g. in SnowflakeSQLException) when it originates from the
     // inner uploadWithDownScopedToken method, which catches the original StorageException and
     // re-wraps it before re-throwing. Walking the chain ensures that a transient 503 or similar
-    // GCS error is always treated as retryable regardless of how many layers of wrapping were added.
+    // GCS error is always treated as retryable regardless of how many layers of wrapping were
+    // added.
     StorageException se = null;
     if (ex instanceof StorageException) {
       se = (StorageException) ex;