From 9524c1fa080dbddeccadbe4e69fa0f9dbdd01115 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Mon, 4 May 2026 11:39:12 +0000 Subject: [PATCH 01/15] feat: add automatic S3 request retry with exponential backoff Implements SDK-level automatic retry matching the minio-go retry-mechanism spec. All requests through BaseS3Client.executeAsync() now retry on retryable S3 error codes (InternalError, SlowDown, RequestTimeout, etc.), retryable HTTP status codes (408, 429, 499, 500, 502, 503, 504, 520), and transient IO errors (connection reset, EOF, server closed idle connection). Non-seekable request bodies (raw okhttp3.RequestBody) get a single attempt; seekable bodies (byte[], ByteBuffer, RandomAccessFile) retry up to maxRetries (default 10). Backoff is full-jitter exponential: rand(0, min(1s, 200ms*2^n)) before the nth retry, matching minio-go's DefaultRetryUnit/DefaultRetryCap. The "RetryHead" S3 code is explicitly excluded so executeHeadAsync region redirect logic is unaffected. maxRetries is configurable via builder and setMaxRetries() post-construction. Closes #1700. --- api/src/main/java/io/minio/BaseS3Client.java | 72 +- api/src/main/java/io/minio/Http.java | 5 + .../main/java/io/minio/MinioAsyncClient.java | 16 +- api/src/main/java/io/minio/MinioClient.java | 18 + api/src/main/java/io/minio/Retry.java | 152 +++++ .../errors/InvalidResponseException.java | 8 + api/src/test/java/io/minio/RetryTest.java | 645 ++++++++++++++++++ 7 files changed, 909 insertions(+), 7 deletions(-) create mode 100644 api/src/main/java/io/minio/Retry.java create mode 100644 api/src/test/java/io/minio/RetryTest.java diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index e47c29325..6feb8def0 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -62,6 +62,9 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.logging.Logger; import javax.annotation.Nonnull; @@ -103,6 +106,15 @@ public abstract class BaseS3Client implements AutoCloseable { private static final String UPLOAD_ID = "uploadId"; private static final Set TRACE_QUERY_PARAMS = ImmutableSet.of("retention", "legal-hold", "tagging", UPLOAD_ID, "acl", "attributes"); + + private static final ScheduledExecutorService RETRY_SCHEDULER = + Executors.newSingleThreadScheduledExecutor( + r -> { + Thread t = new Thread(r, "minio-retry-scheduler"); + t.setDaemon(true); + return t; + }); + private PrintWriter traceStream; protected final Map regionCache = new ConcurrentHashMap<>(); protected String userAgent = Utils.getDefaultUserAgent(); @@ -111,6 +123,7 @@ public abstract class BaseS3Client implements AutoCloseable { protected Provider provider; protected OkHttpClient httpClient; protected boolean closeHttpClient; + protected volatile int maxRetries = Retry.MAX_RETRY; protected BaseS3Client( Http.BaseUrl baseUrl, Provider provider, OkHttpClient httpClient, boolean closeHttpClient) { @@ -125,6 +138,7 @@ protected BaseS3Client(BaseS3Client client) { this.provider = client.provider; this.httpClient = client.httpClient; this.closeHttpClient = client.closeHttpClient; + this.maxRetries = client.maxRetries; } /** Closes underneath HTTP client. */ @@ -136,6 +150,18 @@ public void close() { } } + /** + * Sets the maximum number of retry attempts for failed S3 requests. Requests with non-seekable + * bodies are never retried regardless of this value. The default is {@code Retry.MAX_RETRY} (10). + * Pass 1 to disable automatic retries. + * + * @param maxRetries maximum attempts (must be >= 1). + */ + public void setMaxRetries(int maxRetries) { + if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1"); + this.maxRetries = maxRetries; + } + /** * Sets HTTP connect, write and read timeouts. A value of 0 means no timeout, otherwise values * must be between 1 and Integer.MAX_VALUE when converted to milliseconds. @@ -270,8 +296,47 @@ private String[] handleRedirectResponse( return new String[] {code, message}; } - /** Execute HTTP request asynchronously for given parameters. */ + /** Execute HTTP request asynchronously for given parameters, with automatic retry. */ protected CompletableFuture executeAsync(Http.S3Request s3request, String region) { + // Non-seekable bodies (raw okhttp3 RequestBody) cannot be replayed — single attempt only. + Http.Body body = s3request.body(); + int maxAttempts = (body != null && body.isHttpRequestBody()) ? 1 : this.maxRetries; + return executeWithRetry(s3request, region, maxAttempts, 0); + } + + private CompletableFuture executeWithRetry( + Http.S3Request s3request, String region, int maxAttempts, int attempt) { + return doExecuteAsync(s3request, region) + .handle( + (response, throwable) -> { + if (throwable == null) { + return CompletableFuture.completedFuture(response); + } + Throwable cause = + (throwable instanceof CompletionException) ? throwable.getCause() : throwable; + if (cause == null) cause = throwable; + if (attempt + 1 >= maxAttempts || !Retry.isRetryable(cause)) { + return Utils.failedFuture(cause); + } + long delayMs = Retry.computeBackoffMs(attempt + 1, RANDOM); + CompletableFuture retryFuture = new CompletableFuture<>(); + RETRY_SCHEDULER.schedule( + () -> + executeWithRetry(s3request, region, maxAttempts, attempt + 1) + .whenComplete( + (r, t) -> { + if (t != null) retryFuture.completeExceptionally(t); + else retryFuture.complete(r); + }), + delayMs, + TimeUnit.MILLISECONDS); + return retryFuture; + }) + .thenCompose(cf -> cf); + } + + /** Execute single HTTP request attempt asynchronously for given parameters. */ + private CompletableFuture doExecuteAsync(Http.S3Request s3request, String region) { Credentials credentials = (provider == null) ? null : provider.fetch(); Http.Request request = null; try { @@ -285,11 +350,6 @@ protected CompletableFuture executeAsync(Http.S3Request s3request, Str if (traceStream != null) traceStream.print(request.httpTraces()); OkHttpClient httpClient = this.httpClient; - // FIXME: enable retry for all request. - // if (!s3request.retryFailure()) { - // httpClient = httpClient.newBuilder().retryOnConnectionFailure(false).build(); - // } - okhttp3.Request httpRequest = request.httpRequest(); CompletableFuture completableFuture = newCompleteableFuture(); httpClient diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index 5fa9bec24..9abdcb1d7 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -1503,6 +1503,11 @@ public String object() { return object; } + /** Returns the request body, or {@code null} if none was set. */ + public Body body() { + return body; + } + private Request toRequest( BaseUrl baseUrl, String region, Credentials credentials, Integer expiry) throws MinioException { diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java index dbe7d7578..67ca18bfb 100644 --- a/api/src/main/java/io/minio/MinioAsyncClient.java +++ b/api/src/main/java/io/minio/MinioAsyncClient.java @@ -156,6 +156,7 @@ public static final class Builder { private Provider provider; private OkHttpClient httpClient; private boolean closeHttpClient; + private int maxRetries = Retry.MAX_RETRY; public Builder baseUrl(Http.BaseUrl baseUrl) { if (baseUrl.region() == null) { @@ -217,6 +218,16 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { return this; } + /** + * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries. + * Defaults to {@link Retry#MAX_RETRY} (10). + */ + public Builder maxRetries(int maxRetries) { + if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1"); + this.maxRetries = maxRetries; + return this; + } + public MinioAsyncClient build() { Utils.validateNotNull(baseUrl, "endpoint"); @@ -232,7 +243,10 @@ public MinioAsyncClient build() { httpClient = Http.newDefaultClient(); } - return new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient); + MinioAsyncClient client = + new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient); + client.maxRetries = maxRetries; + return client; } } diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java index d2413f87c..983a3fac7 100644 --- a/api/src/main/java/io/minio/MinioClient.java +++ b/api/src/main/java/io/minio/MinioClient.java @@ -1979,6 +1979,15 @@ public void setAwsS3Prefix(String awsS3Prefix) { asyncClient.setAwsS3Prefix(awsS3Prefix); } + /** + * Sets the maximum number of retry attempts. Pass 1 to disable automatic retries. + * + * @param maxRetries maximum attempts (must be >= 1). + */ + public void setMaxRetries(int maxRetries) { + asyncClient.setMaxRetries(maxRetries); + } + /** Closes underneath async client. */ @Override public void close() throws Exception { @@ -2043,6 +2052,15 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { return this; } + /** + * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries. + * Defaults to {@link Retry#MAX_RETRY} (10). + */ + public Builder maxRetries(int maxRetries) { + asyncClientBuilder.maxRetries(maxRetries); + return this; + } + public MinioClient build() { MinioAsyncClient asyncClient = asyncClientBuilder.build(); return new MinioClient(asyncClient); diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java new file mode 100644 index 000000000..14c0f1a32 --- /dev/null +++ b/api/src/main/java/io/minio/Retry.java @@ -0,0 +1,152 @@ +/* + * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2026 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.minio; + +import com.google.common.collect.ImmutableSet; +import io.minio.errors.ErrorResponseException; +import io.minio.errors.InvalidResponseException; +import io.minio.errors.ServerException; +import java.io.IOException; +import java.util.Random; +import java.util.Set; +import java.util.concurrent.CompletionException; +import javax.net.ssl.SSLHandshakeException; + +/** Retry configuration and helpers for S3 request execution. */ +class Retry { + /** Default maximum number of retry attempts per request. */ + static final int MAX_RETRY = 10; + + /** Base sleep unit for exponential backoff (milliseconds). */ + static final long RETRY_BASE_MS = 200L; + + /** Maximum sleep cap for exponential backoff (milliseconds). */ + static final long RETRY_CAP_MS = 1_000L; + + /** + * S3 error codes that should trigger a retry. Matches the retryableS3Codes set from minio-go + * retry.go. + */ + private static final Set RETRYABLE_S3_CODES = + ImmutableSet.of( + "RequestError", + "RequestTimeout", + "Throttling", + "ThrottlingException", + "RequestLimitExceeded", + "RequestThrottled", + "InternalError", + "ExpiredToken", + "ExpiredTokenException", + "SlowDown", + "SlowDownWrite", + "SlowDownRead"); + + /** + * HTTP status codes that should trigger a retry. Matches retryableHTTPStatusCodes from minio-go + * retry.go. + */ + private static final Set RETRYABLE_HTTP_CODES = + ImmutableSet.of( + 408, // Request Timeout + 429, // Too Many Requests + 499, // Client Closed Request (nginx) + 500, // Internal Server Error + 502, // Bad Gateway + 503, // Service Unavailable + 504, // Gateway Timeout + 520 // Cloudflare unknown error + ); + + static boolean isRetryableS3Code(String code) { + return code != null && RETRYABLE_S3_CODES.contains(code); + } + + static boolean isRetryableHttpCode(int code) { + return RETRYABLE_HTTP_CODES.contains(code); + } + + /** + * Returns true if the IOException is retryable. Non-retryable: TLS handshake failures, HTTP/HTTPS + * protocol mismatch. Everything else (connection reset, EOF, server closed idle connection) is + * retried. + */ + static boolean isRetryableIOException(IOException e) { + // TLS certificate / handshake failures are not retryable. + if (e instanceof SSLHandshakeException) return false; + String msg = e.getMessage(); + // Protocol mismatch is not retryable. + if (msg != null && msg.contains("server gave HTTP response to HTTPS client")) return false; + return true; + } + + /** + * Returns true if the throwable represents a retryable failure. Handles IOException, + * ErrorResponseException, ServerException, and InvalidResponseException. + */ + static boolean isRetryable(Throwable t) { + if (t instanceof CompletionException) t = t.getCause(); + if (t == null) return false; + + if (t instanceof IOException) { + return isRetryableIOException((IOException) t); + } + + if (t instanceof ErrorResponseException) { + ErrorResponseException e = (ErrorResponseException) t; + String code = e.errorResponse().code(); + // "RetryHead" is handled separately by executeHeadAsync — must not be swallowed here. + if ("RetryHead".equals(code)) return false; + if (isRetryableS3Code(code)) return true; + if (e.response() != null && isRetryableHttpCode(e.response().code())) return true; + return false; + } + + if (t instanceof ServerException) { + return isRetryableHttpCode(((ServerException) t).statusCode()); + } + + if (t instanceof InvalidResponseException) { + return isRetryableHttpCode(((InvalidResponseException) t).responseCode()); + } + + return false; + } + + /** + * Computes the full-jitter exponential backoff delay for retry {@code attempt} (1-indexed: 1 = + * first retry). Matches minio-go's {@code exponentialBackoffWait(i)}: + * + *
+   * attempt=1 → [0, 200 ms]
+   * attempt=2 → [0, 400 ms]
+   * attempt=3 → [0, 800 ms]
+   * attempt=4+→ [0, 1000 ms] (capped)
+   * 
+ * + * Pass {@code attempt <= 0} to get 0 (no delay). + */ + static long computeBackoffMs(int attempt, Random random) { + if (attempt <= 0) return 0L; + // exp = attempt-1 so that attempt=1 maps to base*2^0=200ms cap + int exp = Math.min(attempt - 1, 30); + long cap = Math.min(RETRY_CAP_MS, RETRY_BASE_MS * (1L << exp)); + return (long) (random.nextDouble() * cap); + } + + private Retry() {} +} diff --git a/api/src/main/java/io/minio/errors/InvalidResponseException.java b/api/src/main/java/io/minio/errors/InvalidResponseException.java index 1305b3cdd..abf3c225a 100644 --- a/api/src/main/java/io/minio/errors/InvalidResponseException.java +++ b/api/src/main/java/io/minio/errors/InvalidResponseException.java @@ -20,6 +20,8 @@ public class InvalidResponseException extends MinioException { private static final long serialVersionUID = -4793742105569629274L; + private final int responseCode; + public InvalidResponseException( int responseCode, String contentType, String body, String httpTrace) { super( @@ -30,5 +32,11 @@ public InvalidResponseException( + ", body: " + body, httpTrace); + this.responseCode = responseCode; + } + + /** Returns the HTTP response code that triggered this exception. */ + public int responseCode() { + return responseCode; } } diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java new file mode 100644 index 000000000..6ab323b7b --- /dev/null +++ b/api/src/test/java/io/minio/RetryTest.java @@ -0,0 +1,645 @@ +/* + * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2026 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.minio; + +import io.minio.errors.ErrorResponseException; +import io.minio.errors.InvalidResponseException; +import io.minio.errors.ServerException; +import io.minio.messages.ErrorResponse; +import java.io.IOException; +import java.net.SocketException; +import java.util.Random; +import java.util.concurrent.CompletionException; +import javax.net.ssl.SSLHandshakeException; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import okio.Buffer; +import org.junit.Assert; +import org.junit.Test; + +/** Unit and integration tests for the automatic retry mechanism. */ +public class RetryTest { + + // --------------------------------------------------------------------------- + // Retry.isRetryableS3Code tests + // --------------------------------------------------------------------------- + + @Test + public void testIsRetryableS3Code_retryable() { + Assert.assertTrue(Retry.isRetryableS3Code("RequestError")); + Assert.assertTrue(Retry.isRetryableS3Code("RequestTimeout")); + Assert.assertTrue(Retry.isRetryableS3Code("Throttling")); + Assert.assertTrue(Retry.isRetryableS3Code("ThrottlingException")); + Assert.assertTrue(Retry.isRetryableS3Code("RequestLimitExceeded")); + Assert.assertTrue(Retry.isRetryableS3Code("RequestThrottled")); + Assert.assertTrue(Retry.isRetryableS3Code("InternalError")); + Assert.assertTrue(Retry.isRetryableS3Code("ExpiredToken")); + Assert.assertTrue(Retry.isRetryableS3Code("ExpiredTokenException")); + Assert.assertTrue(Retry.isRetryableS3Code("SlowDown")); + Assert.assertTrue(Retry.isRetryableS3Code("SlowDownWrite")); + Assert.assertTrue(Retry.isRetryableS3Code("SlowDownRead")); + } + + @Test + public void testIsRetryableS3Code_notRetryable() { + Assert.assertFalse(Retry.isRetryableS3Code("NoSuchKey")); + Assert.assertFalse(Retry.isRetryableS3Code("NoSuchBucket")); + Assert.assertFalse(Retry.isRetryableS3Code("AccessDenied")); + Assert.assertFalse(Retry.isRetryableS3Code("InvalidBucketName")); + Assert.assertFalse(Retry.isRetryableS3Code("RetryHead")); + Assert.assertFalse(Retry.isRetryableS3Code("MethodNotAllowed")); + Assert.assertFalse(Retry.isRetryableS3Code("PreconditionFailed")); + Assert.assertFalse(Retry.isRetryableS3Code("")); + } + + @Test + public void testIsRetryableS3Code_null() { + Assert.assertFalse(Retry.isRetryableS3Code(null)); + } + + // --------------------------------------------------------------------------- + // Retry.isRetryableHttpCode tests + // --------------------------------------------------------------------------- + + @Test + public void testIsRetryableHttpCode_retryable() { + Assert.assertTrue(Retry.isRetryableHttpCode(408)); + Assert.assertTrue(Retry.isRetryableHttpCode(429)); + Assert.assertTrue(Retry.isRetryableHttpCode(499)); + Assert.assertTrue(Retry.isRetryableHttpCode(500)); + Assert.assertTrue(Retry.isRetryableHttpCode(502)); + Assert.assertTrue(Retry.isRetryableHttpCode(503)); + Assert.assertTrue(Retry.isRetryableHttpCode(504)); + Assert.assertTrue(Retry.isRetryableHttpCode(520)); + } + + @Test + public void testIsRetryableHttpCode_notRetryable() { + Assert.assertFalse(Retry.isRetryableHttpCode(200)); + Assert.assertFalse(Retry.isRetryableHttpCode(201)); + Assert.assertFalse(Retry.isRetryableHttpCode(400)); + Assert.assertFalse(Retry.isRetryableHttpCode(403)); + Assert.assertFalse(Retry.isRetryableHttpCode(404)); + Assert.assertFalse(Retry.isRetryableHttpCode(409)); + Assert.assertFalse(Retry.isRetryableHttpCode(412)); + } + + // --------------------------------------------------------------------------- + // Retry.isRetryableIOException tests + // --------------------------------------------------------------------------- + + @Test + public void testIsRetryableIOException_retryable() { + Assert.assertTrue(Retry.isRetryableIOException(new IOException("connection reset"))); + Assert.assertTrue(Retry.isRetryableIOException(new IOException("EOF"))); + Assert.assertTrue( + Retry.isRetryableIOException(new IOException("http: server closed idle connection"))); + Assert.assertTrue(Retry.isRetryableIOException(new SocketException("Connection timed out"))); + Assert.assertTrue(Retry.isRetryableIOException(new java.net.SocketTimeoutException("read"))); + } + + @Test + public void testIsRetryableIOException_sslHandshakeNotRetryable() { + Assert.assertFalse(Retry.isRetryableIOException(new SSLHandshakeException("cert not trusted"))); + } + + @Test + public void testIsRetryableIOException_protocolMismatchNotRetryable() { + Assert.assertFalse( + Retry.isRetryableIOException(new IOException("server gave HTTP response to HTTPS client"))); + } + + // --------------------------------------------------------------------------- + // Retry.isRetryable(Throwable) tests + // --------------------------------------------------------------------------- + + @Test + public void testIsRetryable_ioException() { + Assert.assertTrue(Retry.isRetryable(new IOException("connection reset"))); + } + + @Test + public void testIsRetryable_ioExceptionWrappedInCompletion() { + Assert.assertTrue( + Retry.isRetryable(new CompletionException(new IOException("connection reset")))); + } + + @Test + public void testIsRetryable_sslHandshakeNotRetryable() { + Assert.assertFalse(Retry.isRetryable(new SSLHandshakeException("bad cert"))); + } + + @Test + public void testIsRetryable_serverException_retryable() throws Exception { + Assert.assertTrue(Retry.isRetryable(new ServerException("Server Error", 500, ""))); + Assert.assertTrue(Retry.isRetryable(new ServerException("Bad Gateway", 502, ""))); + Assert.assertTrue(Retry.isRetryable(new ServerException("Service Unavailable", 503, ""))); + } + + @Test + public void testIsRetryable_serverException_notRetryable() throws Exception { + Assert.assertFalse(Retry.isRetryable(new ServerException("Not Modified", 304, ""))); + } + + @Test + public void testIsRetryable_invalidResponseException_retryable() throws Exception { + Assert.assertTrue(Retry.isRetryable(new InvalidResponseException(500, "text/html", "", ""))); + Assert.assertTrue(Retry.isRetryable(new InvalidResponseException(503, "text/html", "", ""))); + } + + @Test + public void testIsRetryable_invalidResponseException_notRetryable() throws Exception { + Assert.assertFalse(Retry.isRetryable(new InvalidResponseException(403, "text/html", "", ""))); + Assert.assertFalse(Retry.isRetryable(new InvalidResponseException(404, "text/html", "", ""))); + } + + @Test + public void testIsRetryable_null() { + Assert.assertFalse(Retry.isRetryable(null)); + } + + @Test + public void testIsRetryable_retryHeadNotRetryable() throws Exception { + // The explicit guard in isRetryable() must fire even if "RetryHead" were in the codes set. + ErrorResponse errorResponse = + new ErrorResponse("RetryHead", null, null, null, null, null, null); + ErrorResponseException e = new ErrorResponseException(errorResponse, null, ""); + Assert.assertFalse(Retry.isRetryable(e)); + } + + // --------------------------------------------------------------------------- + // Retry.computeBackoffMs tests + // --------------------------------------------------------------------------- + + @Test + public void testComputeBackoffMs_attemptZeroIsZero() { + Random random = new Random(42); + Assert.assertEquals(0L, Retry.computeBackoffMs(0, random)); + } + + @Test + public void testComputeBackoffMs_negativeAttemptIsZero() { + Random random = new Random(42); + Assert.assertEquals(0L, Retry.computeBackoffMs(-1, random)); + } + + @Test + public void testComputeBackoffMs_firstRetryWithinCap() { + // attempt=1 (first retry after one failure): cap = min(1000, 200*2^0) = 200ms + for (int seed = 0; seed < 20; seed++) { + Random random = new Random(seed); + long delay = Retry.computeBackoffMs(1, random); + Assert.assertTrue("first retry delay must be in [0, 200ms]", delay >= 0 && delay <= 200); + } + } + + @Test + public void testComputeBackoffMs_secondRetryWithinCap() { + // attempt=2 (second retry): cap = min(1000, 200*2^1) = 400ms + for (int seed = 0; seed < 20; seed++) { + Random random = new Random(seed); + long delay = Retry.computeBackoffMs(2, random); + Assert.assertTrue("second retry delay must be in [0, 400ms]", delay >= 0 && delay <= 400); + } + } + + @Test + public void testComputeBackoffMs_cappedAtRetryCapMs() { + // With attempt=5, uncapped = 200 * 32 = 6400ms, but should be capped at 1000ms + for (int seed = 0; seed < 20; seed++) { + Random random = new Random(seed); + long delay = Retry.computeBackoffMs(5, random); + Assert.assertTrue( + "delay must be <= RETRY_CAP_MS (" + Retry.RETRY_CAP_MS + ")", + delay <= Retry.RETRY_CAP_MS); + Assert.assertTrue("delay must be >= 0", delay >= 0); + } + } + + @Test + public void testComputeBackoffMs_highAttemptCapped() { + for (int seed = 0; seed < 20; seed++) { + Random random = new Random(seed); + long delay = Retry.computeBackoffMs(100, random); + Assert.assertTrue("delay must be <= RETRY_CAP_MS", delay <= Retry.RETRY_CAP_MS); + } + } + + // --------------------------------------------------------------------------- + // Integration tests via MockWebServer + // --------------------------------------------------------------------------- + + private static final String LIST_BUCKETS_OK = + "" + + "" + + "testtest" + + ""; + + private static final String INTERNAL_ERROR_XML = + "" + + "InternalError" + + "We encountered an internal error. Please try again." + + "/abc"; + + private static final String THROTTLE_XML = + "" + + "SlowDown" + + "Please reduce your request rate." + + "/abc"; + + /** Returns a 200 OK response with valid ListAllMyBucketsResult body. */ + private MockResponse successResponse() { + return new MockResponse() + .setResponseCode(200) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(LIST_BUCKETS_OK)); + } + + /** Returns a 500 server error response with HTML body (non-XML). */ + private MockResponse serverError500Html() { + return new MockResponse() + .setResponseCode(500) + .setHeader("Content-Type", "text/html") + .setBody(new Buffer().writeUtf8("Internal Server Error")); + } + + /** Returns a 503 error with HTML body. */ + private MockResponse serviceUnavailable503() { + return new MockResponse() + .setResponseCode(503) + .setHeader("Content-Type", "text/html") + .setBody(new Buffer().writeUtf8("Service Unavailable")); + } + + /** Returns a 500 S3 InternalError XML response. */ + private MockResponse s3InternalErrorResponse() { + return new MockResponse() + .setResponseCode(500) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(INTERNAL_ERROR_XML)); + } + + /** Returns a 429 TooManyRequests with S3 SlowDown XML. */ + private MockResponse s3ThrottleResponse() { + return new MockResponse() + .setResponseCode(429) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(THROTTLE_XML)); + } + + @Test + public void testRetryOn500HtmlThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + // First request → 500 HTML (non-XML) → InvalidResponseException(500) → retryable + server.enqueue(serverError500Html()); + // Second request (retry) → 200 OK + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + // Should succeed after one retry + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRetryOn503ThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(serviceUnavailable503()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRetryOnS3InternalErrorThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(s3InternalErrorResponse()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRetryOnS3ThrottleThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(s3ThrottleResponse()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testNoRetryOn404() throws Exception { + String notFoundXml = + "" + + "NoSuchBucketnot found" + + "/testabc"; + + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(404) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(notFoundXml))); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (Exception e) { + Assert.assertNotNull("expected non-null exception", e); + } + // Must not retry — only 1 request should have been made + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testNoRetryOn403() throws Exception { + String accessDeniedXml = + "" + + "AccessDeniedAccess Denied" + + "/abc"; + + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(403) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(accessDeniedXml))); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (Exception e) { + Assert.assertNotNull("expected non-null exception", e); + } + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testRetryExhaustedThrowsLastError() throws Exception { + try (MockWebServer server = new MockWebServer()) { + // All 3 attempts fail with 500 + server.enqueue(serverError500Html()); + server.enqueue(serverError500Html()); + server.enqueue(serverError500Html()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected exception after exhausted retries"); + } catch (Exception e) { + Assert.assertNotNull("expected non-null exception after exhausted retries", e); + } + Assert.assertEquals(3, server.getRequestCount()); + } + } + + @Test + public void testMaxRetriesOneDisablesRetry() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(serverError500Html()); + // Second response should never be reached + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(1).build(); + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (Exception e) { + Assert.assertNotNull("expected non-null exception", e); + } + // maxRetries=1 means a single attempt; must not retry + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testSetMaxRetriesPostConstruction() throws Exception { + try (MockWebServer server = new MockWebServer()) { + // Two failures enqueued, but maxRetries will be set to 1 after construction + server.enqueue(serverError500Html()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.setMaxRetries(1); // override to disable retries + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (Exception e) { + Assert.assertNotNull("expected non-null exception", e); + } + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testMultipleRetrySucceedsOnThirdAttempt() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(serverError500Html()); + server.enqueue(serviceUnavailable503()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + client.listBuckets(); + + Assert.assertEquals(3, server.getRequestCount()); + } + } + + @Test + public void testRetry429ThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(429) + .setHeader("Content-Type", "text/html") + .setBody(new Buffer().writeUtf8("Too Many Requests"))); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRetry502ThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(502) + .setHeader("Content-Type", "text/html") + .setBody(new Buffer().writeUtf8("Bad Gateway"))); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRetry504ThenSuccess() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(504) + .setHeader("Content-Type", "text/html") + .setBody(new Buffer().writeUtf8("Gateway Timeout"))); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testMaxRetriesBuilderValidation() { + MinioClient.builder().endpoint("http://localhost:9000").maxRetries(0).build(); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetMaxRetriesValidation() throws Exception { + MinioClient client = MinioClient.builder().endpoint("http://localhost:9000").build(); + client.setMaxRetries(0); + } + + @Test + public void testDefaultMaxRetriesIsConfigurable() throws Exception { + // Use maxRetries=4 to keep test fast (delays ≤ 200+400+800ms ≈ 1.4s max) + int retries = 4; + try (MockWebServer server = new MockWebServer()) { + for (int i = 0; i < retries - 1; i++) { + server.enqueue(serverError500Html()); + } + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(retries).build(); + client.listBuckets(); + + Assert.assertEquals(retries, server.getRequestCount()); + } + } + + @Test + public void testRequestBodyRetried() throws Exception { + // Byte array body (seekable) should be retried + try (MockWebServer server = new MockWebServer()) { + // First putObject fails, second succeeds with ETag + server.enqueue(serverError500Html()); + server.enqueue( + new MockResponse() + .setResponseCode(200) + .setHeader("ETag", "\"abc123\"") + .setHeader("Content-Length", "0")); + server.start(); + + // Specify region so the SDK doesn't need a separate GetBucketLocation request + MinioClient client = + MinioClient.builder() + .endpoint(server.url("").toString()) + .credentials("access", "secret") + .region("us-east-1") + .maxRetries(2) + .build(); + + client.putObject( + PutObjectArgs.builder() + .bucket("test-bucket") + .object("test-object") + .data(new byte[0], 0) + .build()); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testRecordedRequestHeaders() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(serverError500Html()); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + // Both requests should be to the same path + RecordedRequest first = server.takeRequest(); + RecordedRequest second = server.takeRequest(); + Assert.assertEquals(first.getPath(), second.getPath()); + } + } + + @Test + public void testInvalidResponseExceptionHasResponseCode() { + InvalidResponseException e = new InvalidResponseException(503, "text/html", "body", "trace"); + Assert.assertEquals(503, e.responseCode()); + } +} From a79aa11332355bd8d8ec27beaa821194074c24ae Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 15:38:27 +0000 Subject: [PATCH 02/15] fix: address Copilot review on retry mechanism - Http.Body now captures the initial file pointer (fileOffset) when created for RandomAccessFile bodies; toRequestBody() seeks back to that offset before each attempt, fixing both first-attempt and retry payload corruption when checksum computation has advanced the pointer - BaseS3Client.createBody() explicitly restores the file pointer after computing checksums so Http.Body captures the correct starting offset - Retry scheduler now dispatches actual retry work (credential fetch + request build) to ForkJoinPool.commonPool(), freeing the single scheduler thread from head-of-line blocking by slow providers - Replace package-private {@link Retry#MAX_RETRY} Javadoc references in MinioClient and MinioAsyncClient with {@code 10} to satisfy doclint - Fix RETRYABLE_HTTP_CODES closing paren indentation to match codebase style (ImmutableSet.of() in Http.java and Signer.java) - Add testRandomAccessFileBodyRetried integration test: verifies that both the failed and retried PUT requests carry identical body content --- api/src/main/java/io/minio/BaseS3Client.java | 31 ++++++++++--- api/src/main/java/io/minio/Http.java | 15 ++++++- .../main/java/io/minio/MinioAsyncClient.java | 2 +- api/src/main/java/io/minio/MinioClient.java | 2 +- api/src/main/java/io/minio/Retry.java | 3 +- api/src/test/java/io/minio/RetryTest.java | 45 +++++++++++++++++++ 6 files changed, 87 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 6feb8def0..4bc33b8a7 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -322,12 +322,14 @@ private CompletableFuture executeWithRetry( CompletableFuture retryFuture = new CompletableFuture<>(); RETRY_SCHEDULER.schedule( () -> - executeWithRetry(s3request, region, maxAttempts, attempt + 1) - .whenComplete( - (r, t) -> { - if (t != null) retryFuture.completeExceptionally(t); - else retryFuture.complete(r); - }), + CompletableFuture.runAsync( + () -> + executeWithRetry(s3request, region, maxAttempts, attempt + 1) + .whenComplete( + (r, t) -> { + if (t != null) retryFuture.completeExceptionally(t); + else retryFuture.complete(r); + })), delayMs, TimeUnit.MILLISECONDS); return retryFuture; @@ -1285,6 +1287,15 @@ private Object[] createBody(PutObjectAPIBaseArgs args, MediaType contentType) boolean checksumHeader = headers.namePrefixAny("x-amz-checksum-"); String md5Hash = headers.getFirst(Http.Headers.CONTENT_MD5); + long fileStartPos = 0; + if (args.file() != null) { + try { + fileStartPos = args.file().getFilePointer(); + } catch (IOException e) { + throw new MinioException(e); + } + } + if (sha256HexString == null && sha256Base64String == null) { if (!baseUrl.isHttps()) { Checksum.Hasher hasher = Checksum.Algorithm.SHA256.hasher(); @@ -1340,6 +1351,14 @@ private Object[] createBody(PutObjectAPIBaseArgs args, MediaType contentType) } } + if (args.file() != null) { + try { + args.file().seek(fileStartPos); + } catch (IOException e) { + throw new MinioException(e); + } + } + Http.Body body = null; if (args.file() != null) { body = new Http.Body(args.file(), args.length(), contentType, sha256HexString, md5Hash); diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index 9abdcb1d7..ad25632b2 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -690,6 +690,7 @@ public static OkHttpClient setTimeout( public static class Body { private okhttp3.RequestBody requestBody; private RandomAccessFile file; + private long fileOffset; private ByteBuffer buffer; private byte[] data; private Long length; @@ -713,6 +714,11 @@ public Body( String md5Hash) { if (length < 0) throw new IllegalArgumentException("valid length must be provided"); this.file = file; + try { + this.fileOffset = file.getFilePointer(); + } catch (IOException e) { + throw new IllegalStateException("failed to read file position", e); + } set(length, contentType, sha256Hash, md5Hash); } @@ -786,7 +792,14 @@ public Headers headers() { /** Creates HTTP RequestBody for this body. */ public RequestBody toRequestBody() throws MinioException { if (requestBody != null) return new RequestBody(requestBody); - if (file != null) return new RequestBody(file, length, contentType); + if (file != null) { + try { + file.seek(fileOffset); + } catch (IOException e) { + throw new MinioException(e); + } + return new RequestBody(file, length, contentType); + } if (buffer != null) return new RequestBody(buffer, contentType); return new RequestBody(data, length.intValue(), contentType); } diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java index 67ca18bfb..17ec0641f 100644 --- a/api/src/main/java/io/minio/MinioAsyncClient.java +++ b/api/src/main/java/io/minio/MinioAsyncClient.java @@ -220,7 +220,7 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { /** * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries. - * Defaults to {@link Retry#MAX_RETRY} (10). + * Defaults to {@code 10}. */ public Builder maxRetries(int maxRetries) { if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1"); diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java index 983a3fac7..1021f7e64 100644 --- a/api/src/main/java/io/minio/MinioClient.java +++ b/api/src/main/java/io/minio/MinioClient.java @@ -2054,7 +2054,7 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { /** * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries. - * Defaults to {@link Retry#MAX_RETRY} (10). + * Defaults to {@code 10}. */ public Builder maxRetries(int maxRetries) { asyncClientBuilder.maxRetries(maxRetries); diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java index 14c0f1a32..cd1dcfe84 100644 --- a/api/src/main/java/io/minio/Retry.java +++ b/api/src/main/java/io/minio/Retry.java @@ -69,8 +69,7 @@ class Retry { 502, // Bad Gateway 503, // Service Unavailable 504, // Gateway Timeout - 520 // Cloudflare unknown error - ); + 520); // Cloudflare unknown error static boolean isRetryableS3Code(String code) { return code != null && RETRYABLE_S3_CODES.contains(code); diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 6ab323b7b..c18b0d6e7 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -21,7 +21,10 @@ import io.minio.errors.ServerException; import io.minio.messages.ErrorResponse; import java.io.IOException; +import java.io.RandomAccessFile; import java.net.SocketException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Random; import java.util.concurrent.CompletionException; import javax.net.ssl.SSLHandshakeException; @@ -619,6 +622,48 @@ public void testRequestBodyRetried() throws Exception { } } + @Test + public void testRandomAccessFileBodyRetried() throws Exception { + Path tmpFile = Files.createTempFile("retry-raf-test", ".bin"); + try (MockWebServer server = new MockWebServer()) { + server.enqueue(serverError500Html()); + server.enqueue( + new MockResponse() + .setResponseCode(200) + .setHeader("ETag", "\"abc123\"") + .setHeader("Content-Length", "0")); + server.start(); + + byte[] content = "hello-retry".getBytes(java.nio.charset.StandardCharsets.UTF_8); + Files.write(tmpFile, content); + + try (RandomAccessFile raf = new RandomAccessFile(tmpFile.toFile(), "r")) { + MinioAsyncClient client = + MinioAsyncClient.builder() + .endpoint(server.url("").toString()) + .credentials("access", "secret") + .region("us-east-1") + .maxRetries(2) + .build(); + client + .putObject( + PutObjectAPIArgs.builder() + .bucket("test-bucket") + .object("test-object") + .file(raf, content.length) + .build()) + .join(); + } + + Assert.assertEquals(2, server.getRequestCount()); + byte[] first = server.takeRequest().getBody().readByteArray(); + byte[] second = server.takeRequest().getBody().readByteArray(); + Assert.assertArrayEquals(first, second); + } finally { + Files.deleteIfExists(tmpFile); + } + } + @Test public void testRecordedRequestHeaders() throws Exception { try (MockWebServer server = new MockWebServer()) { From beead6e762b1a6e2d116f936bc83f1c02c6f040f Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 19:10:23 +0000 Subject: [PATCH 03/15] fix: strengthen exception assertions in RetryTest and guard regionCache.remove The no-retry tests for 404/403 were asserting only assertNotNull, which would pass for any exception including NPEs. Stronger assertions now verify ErrorResponseException type and S3 error code/HTTP status, and the retry-exhausted test verifies InvalidResponseException with status 500. While adding those assertions, the tests revealed a pre-existing NPE: regionCache.remove(s3request.bucket()) was called unconditionally when the S3 error code was NoSuchBucket/RetryHead, but bucket is null for operations like listBuckets(). Added a null guard so the cache invalidation is skipped when there is no bucket in the request. --- api/src/main/java/io/minio/BaseS3Client.java | 7 ++++--- api/src/test/java/io/minio/RetryTest.java | 14 +++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 4bc33b8a7..f7ea43928 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -533,9 +533,10 @@ private void onResponse(final Response response) throws IOException { response.header("x-amz-id-2")); } - // invalidate region cache if needed - if (errorResponse.code().equals(NO_SUCH_BUCKET) - || errorResponse.code().equals(RETRY_HEAD)) { + // invalidate region cache if needed (bucket may be null for e.g. listBuckets) + if (s3request.bucket() != null + && (errorResponse.code().equals(NO_SUCH_BUCKET) + || errorResponse.code().equals(RETRY_HEAD))) { regionCache.remove(s3request.bucket()); } diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index c18b0d6e7..5b5b456a7 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -389,7 +389,10 @@ public void testNoRetryOn404() throws Exception { client.listBuckets(); Assert.fail("expected exception"); } catch (Exception e) { - Assert.assertNotNull("expected non-null exception", e); + Assert.assertTrue("expected ErrorResponseException", e instanceof ErrorResponseException); + ErrorResponseException ere = (ErrorResponseException) e; + Assert.assertEquals("NoSuchBucket", ere.errorResponse().code()); + Assert.assertEquals(404, ere.response().code()); } // Must not retry — only 1 request should have been made Assert.assertEquals(1, server.getRequestCount()); @@ -417,7 +420,10 @@ public void testNoRetryOn403() throws Exception { client.listBuckets(); Assert.fail("expected exception"); } catch (Exception e) { - Assert.assertNotNull("expected non-null exception", e); + Assert.assertTrue("expected ErrorResponseException", e instanceof ErrorResponseException); + ErrorResponseException ere = (ErrorResponseException) e; + Assert.assertEquals("AccessDenied", ere.errorResponse().code()); + Assert.assertEquals(403, ere.response().code()); } Assert.assertEquals(1, server.getRequestCount()); } @@ -438,7 +444,9 @@ public void testRetryExhaustedThrowsLastError() throws Exception { client.listBuckets(); Assert.fail("expected exception after exhausted retries"); } catch (Exception e) { - Assert.assertNotNull("expected non-null exception after exhausted retries", e); + Assert.assertTrue( + "expected InvalidResponseException", e instanceof InvalidResponseException); + Assert.assertEquals(500, ((InvalidResponseException) e).responseCode()); } Assert.assertEquals(3, server.getRequestCount()); } From 53da859a70f13963f24b72be7267a0443e373708 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 19:18:37 +0000 Subject: [PATCH 04/15] fix: replace instanceof assertions with typed catch to satisfy SpotBugs JUA SpotBugs JUA flags Assert.assertTrue(e instanceof T) because the message hides what type was actually thrown on failure. Using a typed catch block is stricter: any unexpected exception type propagates as a test error with the full stack trace, which is more informative than a failed boolean assertion. --- api/src/test/java/io/minio/RetryTest.java | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 5b5b456a7..1559b9901 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -388,11 +388,9 @@ public void testNoRetryOn404() throws Exception { try { client.listBuckets(); Assert.fail("expected exception"); - } catch (Exception e) { - Assert.assertTrue("expected ErrorResponseException", e instanceof ErrorResponseException); - ErrorResponseException ere = (ErrorResponseException) e; - Assert.assertEquals("NoSuchBucket", ere.errorResponse().code()); - Assert.assertEquals(404, ere.response().code()); + } catch (ErrorResponseException e) { + Assert.assertEquals("NoSuchBucket", e.errorResponse().code()); + Assert.assertEquals(404, e.response().code()); } // Must not retry — only 1 request should have been made Assert.assertEquals(1, server.getRequestCount()); @@ -419,11 +417,9 @@ public void testNoRetryOn403() throws Exception { try { client.listBuckets(); Assert.fail("expected exception"); - } catch (Exception e) { - Assert.assertTrue("expected ErrorResponseException", e instanceof ErrorResponseException); - ErrorResponseException ere = (ErrorResponseException) e; - Assert.assertEquals("AccessDenied", ere.errorResponse().code()); - Assert.assertEquals(403, ere.response().code()); + } catch (ErrorResponseException e) { + Assert.assertEquals("AccessDenied", e.errorResponse().code()); + Assert.assertEquals(403, e.response().code()); } Assert.assertEquals(1, server.getRequestCount()); } @@ -443,10 +439,8 @@ public void testRetryExhaustedThrowsLastError() throws Exception { try { client.listBuckets(); Assert.fail("expected exception after exhausted retries"); - } catch (Exception e) { - Assert.assertTrue( - "expected InvalidResponseException", e instanceof InvalidResponseException); - Assert.assertEquals(500, ((InvalidResponseException) e).responseCode()); + } catch (InvalidResponseException e) { + Assert.assertEquals(500, e.responseCode()); } Assert.assertEquals(3, server.getRequestCount()); } From 60aeaed4d600969942b6f63fee3895b548c2689a Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 20:24:26 +0000 Subject: [PATCH 05/15] fix: use ThreadLocalRandom for backoff jitter, disable OkHttp retry, strengthen test assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the shared static Random instance with ThreadLocalRandom.current() at the backoff call site and for the fan-out name generator in MinioAsyncClient — eliminating shared-state CAS contention under concurrent retry load. Disable OkHttp's retryOnConnectionFailure in doExecuteAsync so the SDK's executeWithRetry is the sole retry policy; previously OkHttp could silently add an extra attempt on stale-connection IOExceptions, causing more total attempts than maxRetries. Strengthen the two remaining assertNotNull catch blocks in RetryTest (testMaxRetriesOneDisablesRetry, testSetMaxRetriesPostConstruction) to typed InvalidResponseException catches with responseCode()==500 assertions, matching the pattern applied to the other tests in the previous commit. --- api/src/main/java/io/minio/BaseS3Client.java | 9 ++++----- api/src/main/java/io/minio/MinioAsyncClient.java | 3 ++- api/src/test/java/io/minio/RetryTest.java | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index f7ea43928..187aab598 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -51,17 +51,16 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.nio.charset.StandardCharsets; -import java.security.SecureRandom; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.Random; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -94,7 +93,6 @@ public abstract class BaseS3Client implements AutoCloseable { "ServerSideEncryptionConfigurationNotFoundError"; // maximum allowed bucket policy size is 20KiB protected static final int MAX_BUCKET_POLICY_SIZE = 20 * 1024; - protected static final Random RANDOM = new Random(new SecureRandom().nextLong()); protected static final ObjectMapper OBJECT_MAPPER = JsonMapper.builder() .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) @@ -318,7 +316,7 @@ private CompletableFuture executeWithRetry( if (attempt + 1 >= maxAttempts || !Retry.isRetryable(cause)) { return Utils.failedFuture(cause); } - long delayMs = Retry.computeBackoffMs(attempt + 1, RANDOM); + long delayMs = Retry.computeBackoffMs(attempt + 1, ThreadLocalRandom.current()); CompletableFuture retryFuture = new CompletableFuture<>(); RETRY_SCHEDULER.schedule( () -> @@ -351,7 +349,8 @@ private CompletableFuture doExecuteAsync(Http.S3Request s3request, Str PrintWriter traceStream = this.traceStream; if (traceStream != null) traceStream.print(request.httpTraces()); - OkHttpClient httpClient = this.httpClient; + OkHttpClient httpClient = + this.httpClient.newBuilder().retryOnConnectionFailure(false).build(); okhttp3.Request httpRequest = request.httpRequest(); CompletableFuture completableFuture = newCompleteableFuture(); httpClient diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java index 17ec0641f..c54c8da06 100644 --- a/api/src/main/java/io/minio/MinioAsyncClient.java +++ b/api/src/main/java/io/minio/MinioAsyncClient.java @@ -93,6 +93,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; import okhttp3.HttpUrl; import okhttp3.MediaType; @@ -3370,7 +3371,7 @@ public CompletableFuture putObjectFanOut(PutObjectFanOu // Build POST object data String objectName = "fan-out-" - + new BigInteger(32, RANDOM).toString(32) + + new BigInteger(32, ThreadLocalRandom.current()).toString(32) + "-" + System.currentTimeMillis(); PostPolicy policy = diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 1559b9901..dcfa9e9c9 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -459,8 +459,8 @@ public void testMaxRetriesOneDisablesRetry() throws Exception { try { client.listBuckets(); Assert.fail("expected exception"); - } catch (Exception e) { - Assert.assertNotNull("expected non-null exception", e); + } catch (InvalidResponseException e) { + Assert.assertEquals(500, e.responseCode()); } // maxRetries=1 means a single attempt; must not retry Assert.assertEquals(1, server.getRequestCount()); @@ -481,8 +481,8 @@ public void testSetMaxRetriesPostConstruction() throws Exception { try { client.listBuckets(); Assert.fail("expected exception"); - } catch (Exception e) { - Assert.assertNotNull("expected non-null exception", e); + } catch (InvalidResponseException e) { + Assert.assertEquals(500, e.responseCode()); } Assert.assertEquals(1, server.getRequestCount()); } From d1c52e0512a9512772c1f142fddef0de6abb7f68 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 20:37:11 +0000 Subject: [PATCH 06/15] style: fix Spotless formatting violations in BaseS3Client Reorder ThreadLocalRandom import to match alphabetical sort order and collapse the two-line OkHttpClient builder call to one line, satisfying the project's spotlessJavaCheck formatter requirements. --- api/src/main/java/io/minio/BaseS3Client.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 187aab598..0e4ab9209 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -60,9 +60,9 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.logging.Logger; @@ -349,8 +349,7 @@ private CompletableFuture doExecuteAsync(Http.S3Request s3request, Str PrintWriter traceStream = this.traceStream; if (traceStream != null) traceStream.print(request.httpTraces()); - OkHttpClient httpClient = - this.httpClient.newBuilder().retryOnConnectionFailure(false).build(); + OkHttpClient httpClient = this.httpClient.newBuilder().retryOnConnectionFailure(false).build(); okhttp3.Request httpRequest = request.httpRequest(); CompletableFuture completableFuture = newCompleteableFuture(); httpClient From 0d687ecad7aa5098d4021473932c0354fecd4856 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Tue, 5 May 2026 20:42:55 +0000 Subject: [PATCH 07/15] fix: propagate runAsync dispatch failure into retryFuture If ForkJoinPool.commonPool() rejects the retry task (e.g. during JVM shutdown), the runAsync future completes exceptionally but retryFuture was never completed, causing the overall request future to hang. Wire an .exceptionally() handler on the runAsync future so any dispatch failure propagates into retryFuture immediately. --- api/src/main/java/io/minio/BaseS3Client.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 0e4ab9209..358ba37ea 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -321,13 +321,18 @@ private CompletableFuture executeWithRetry( RETRY_SCHEDULER.schedule( () -> CompletableFuture.runAsync( - () -> - executeWithRetry(s3request, region, maxAttempts, attempt + 1) - .whenComplete( - (r, t) -> { - if (t != null) retryFuture.completeExceptionally(t); - else retryFuture.complete(r); - })), + () -> + executeWithRetry(s3request, region, maxAttempts, attempt + 1) + .whenComplete( + (r, t) -> { + if (t != null) retryFuture.completeExceptionally(t); + else retryFuture.complete(r); + })) + .exceptionally( + ex -> { + retryFuture.completeExceptionally(ex); + return null; + }), delayMs, TimeUnit.MILLISECONDS); return retryFuture; From 34abd6472d59576684b585e240e6a627ebf30ae3 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 7 May 2026 16:16:48 +0000 Subject: [PATCH 08/15] refactor: move retry to OkHttp interceptor, drop S3-code retry Per @balamurugana's review on PR #1701, replace the CompletableFuture/scheduler-based retry orchestration with an OkHttp interceptor (Http.RetryInterceptor) that retries on retryable HTTP status codes (408/429/499/500/502/503/504/520) and IOExceptions with exponential backoff and full jitter. - Drop S3-error-code retry classification; well-behaved S3/MinIO servers return retryable conditions with non-2xx HTTP status, so RETRYABLE_S3_CODES is redundant. - Drop RETRY_SCHEDULER and the executeWithRetry recursive chain. - Non-replayable bodies (raw okhttp3.RequestBody) are sent once via Http.RequestBody.isReplayable(); byte[]/ByteBuffer/RandomAccessFile bodies replay safely because writeTo rewinds the source. - Wire interceptor at construction (BaseS3Client.wrapWithRetry) so setMaxRetries() takes effect on subsequent requests. --- api/src/main/java/io/minio/BaseS3Client.java | 84 +++------- api/src/main/java/io/minio/Http.java | 68 ++++++++ api/src/main/java/io/minio/Retry.java | 72 +-------- api/src/test/java/io/minio/RetryTest.java | 158 +------------------ 4 files changed, 93 insertions(+), 289 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 358ba37ea..00762efca 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -60,10 +60,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadLocalRandom; -import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.logging.Logger; import javax.annotation.Nonnull; @@ -105,14 +101,6 @@ public abstract class BaseS3Client implements AutoCloseable { private static final Set TRACE_QUERY_PARAMS = ImmutableSet.of("retention", "legal-hold", "tagging", UPLOAD_ID, "acl", "attributes"); - private static final ScheduledExecutorService RETRY_SCHEDULER = - Executors.newSingleThreadScheduledExecutor( - r -> { - Thread t = new Thread(r, "minio-retry-scheduler"); - t.setDaemon(true); - return t; - }); - private PrintWriter traceStream; protected final Map regionCache = new ConcurrentHashMap<>(); protected String userAgent = Utils.getDefaultUserAgent(); @@ -127,16 +115,30 @@ protected BaseS3Client( Http.BaseUrl baseUrl, Provider provider, OkHttpClient httpClient, boolean closeHttpClient) { this.baseUrl = baseUrl; this.provider = provider; - this.httpClient = httpClient; this.closeHttpClient = closeHttpClient; + this.httpClient = wrapWithRetry(httpClient); } protected BaseS3Client(BaseS3Client client) { this.baseUrl = client.baseUrl; this.provider = client.provider; - this.httpClient = client.httpClient; this.closeHttpClient = client.closeHttpClient; this.maxRetries = client.maxRetries; + this.httpClient = wrapWithRetry(client.httpClient); + } + + /** + * Adds the retry interceptor to {@code client}. If {@code client} already has one (e.g. from a + * prior wrap via copy constructor), it is replaced so the interceptor reads from this instance's + * mutable {@code maxRetries} field. + */ + private OkHttpClient wrapWithRetry(OkHttpClient client) { + OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false); + builder.interceptors().removeIf(i -> i instanceof Http.RetryInterceptor); + return builder + .addInterceptor( + new Http.RetryInterceptor(() -> this.maxRetries, Retry.RETRYABLE_HTTP_CODES)) + .build(); } /** Closes underneath HTTP client. */ @@ -294,54 +296,11 @@ private String[] handleRedirectResponse( return new String[] {code, message}; } - /** Execute HTTP request asynchronously for given parameters, with automatic retry. */ + /** + * Execute HTTP request asynchronously for given parameters. Network-level retry (transient HTTP + * status codes and IOExceptions) is handled by {@link Http.RetryInterceptor} on the client. + */ protected CompletableFuture executeAsync(Http.S3Request s3request, String region) { - // Non-seekable bodies (raw okhttp3 RequestBody) cannot be replayed — single attempt only. - Http.Body body = s3request.body(); - int maxAttempts = (body != null && body.isHttpRequestBody()) ? 1 : this.maxRetries; - return executeWithRetry(s3request, region, maxAttempts, 0); - } - - private CompletableFuture executeWithRetry( - Http.S3Request s3request, String region, int maxAttempts, int attempt) { - return doExecuteAsync(s3request, region) - .handle( - (response, throwable) -> { - if (throwable == null) { - return CompletableFuture.completedFuture(response); - } - Throwable cause = - (throwable instanceof CompletionException) ? throwable.getCause() : throwable; - if (cause == null) cause = throwable; - if (attempt + 1 >= maxAttempts || !Retry.isRetryable(cause)) { - return Utils.failedFuture(cause); - } - long delayMs = Retry.computeBackoffMs(attempt + 1, ThreadLocalRandom.current()); - CompletableFuture retryFuture = new CompletableFuture<>(); - RETRY_SCHEDULER.schedule( - () -> - CompletableFuture.runAsync( - () -> - executeWithRetry(s3request, region, maxAttempts, attempt + 1) - .whenComplete( - (r, t) -> { - if (t != null) retryFuture.completeExceptionally(t); - else retryFuture.complete(r); - })) - .exceptionally( - ex -> { - retryFuture.completeExceptionally(ex); - return null; - }), - delayMs, - TimeUnit.MILLISECONDS); - return retryFuture; - }) - .thenCompose(cf -> cf); - } - - /** Execute single HTTP request attempt asynchronously for given parameters. */ - private CompletableFuture doExecuteAsync(Http.S3Request s3request, String region) { Credentials credentials = (provider == null) ? null : provider.fetch(); Http.Request request = null; try { @@ -354,10 +313,9 @@ private CompletableFuture doExecuteAsync(Http.S3Request s3request, Str PrintWriter traceStream = this.traceStream; if (traceStream != null) traceStream.print(request.httpTraces()); - OkHttpClient httpClient = this.httpClient.newBuilder().retryOnConnectionFailure(false).build(); okhttp3.Request httpRequest = request.httpRequest(); CompletableFuture completableFuture = newCompleteableFuture(); - httpClient + this.httpClient .newCall(httpRequest) .enqueue( new Callback() { diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index ad25632b2..f0e44c132 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -52,7 +52,9 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; +import java.util.function.IntSupplier; import java.util.regex.Matcher; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -65,6 +67,7 @@ import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; +import okhttp3.Interceptor; import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Protocol; @@ -686,6 +689,63 @@ public static OkHttpClient setTimeout( .build(); } + /** + * OkHttp interceptor that retries requests on retryable HTTP status codes and IOExceptions with + * exponential backoff and full jitter. Requests with non-replayable bodies are sent once. + */ + public static class RetryInterceptor implements Interceptor { + private final IntSupplier maxRetriesSupplier; + private final Set retryStatusCodes; + + public RetryInterceptor(IntSupplier maxRetriesSupplier, Set retryStatusCodes) { + this.maxRetriesSupplier = Objects.requireNonNull(maxRetriesSupplier, "maxRetriesSupplier"); + this.retryStatusCodes = Objects.requireNonNull(retryStatusCodes, "retryStatusCodes"); + } + + private static boolean isReplayable(okhttp3.RequestBody body) { + if (body == null) return true; + if (body instanceof RequestBody) return ((RequestBody) body).isReplayable(); + return false; + } + + @Override + public okhttp3.Response intercept(Chain chain) throws IOException { + okhttp3.Request request = chain.request(); + int maxAttempts = + isReplayable(request.body()) ? Math.max(1, maxRetriesSupplier.getAsInt()) : 1; + + okhttp3.Response response = null; + IOException lastException = null; + for (int attempt = 0; attempt < maxAttempts; attempt++) { + if (attempt > 0) { + long delayMs = Retry.computeBackoffMs(attempt, ThreadLocalRandom.current()); + if (delayMs > 0) { + try { + Thread.sleep(delayMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new IOException("retry interrupted", ie); + } + } + if (response != null) response.close(); + } + + try { + response = chain.proceed(request); + if (!retryStatusCodes.contains(response.code())) return response; + lastException = null; + } catch (IOException e) { + if (!Retry.isRetryableIOException(e)) throw e; + lastException = e; + response = null; + } + } + + if (lastException != null) throw lastException; + return response; + } + } + /** HTTP body of {@link RandomAccessFile}, {@link ByteBuffer} or {@link byte} array. */ public static class Body { private okhttp3.RequestBody requestBody; @@ -893,6 +953,14 @@ public void writeTo(BufferedSink sink) throws IOException { sink.write(bytes, 0, (int) length); } } + + /** + * Returns true if {@link #writeTo} can be invoked multiple times. False for raw okhttp3 + * RequestBody since it may consume a one-shot source. + */ + public boolean isReplayable() { + return body == null; + } } /** HTTP methods. */ diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java index cd1dcfe84..f67ba10f7 100644 --- a/api/src/main/java/io/minio/Retry.java +++ b/api/src/main/java/io/minio/Retry.java @@ -17,13 +17,9 @@ package io.minio; import com.google.common.collect.ImmutableSet; -import io.minio.errors.ErrorResponseException; -import io.minio.errors.InvalidResponseException; -import io.minio.errors.ServerException; import java.io.IOException; import java.util.Random; import java.util.Set; -import java.util.concurrent.CompletionException; import javax.net.ssl.SSLHandshakeException; /** Retry configuration and helpers for S3 request execution. */ @@ -37,30 +33,8 @@ class Retry { /** Maximum sleep cap for exponential backoff (milliseconds). */ static final long RETRY_CAP_MS = 1_000L; - /** - * S3 error codes that should trigger a retry. Matches the retryableS3Codes set from minio-go - * retry.go. - */ - private static final Set RETRYABLE_S3_CODES = - ImmutableSet.of( - "RequestError", - "RequestTimeout", - "Throttling", - "ThrottlingException", - "RequestLimitExceeded", - "RequestThrottled", - "InternalError", - "ExpiredToken", - "ExpiredTokenException", - "SlowDown", - "SlowDownWrite", - "SlowDownRead"); - - /** - * HTTP status codes that should trigger a retry. Matches retryableHTTPStatusCodes from minio-go - * retry.go. - */ - private static final Set RETRYABLE_HTTP_CODES = + /** HTTP status codes that should trigger a retry. */ + static final Set RETRYABLE_HTTP_CODES = ImmutableSet.of( 408, // Request Timeout 429, // Too Many Requests @@ -71,10 +45,6 @@ class Retry { 504, // Gateway Timeout 520); // Cloudflare unknown error - static boolean isRetryableS3Code(String code) { - return code != null && RETRYABLE_S3_CODES.contains(code); - } - static boolean isRetryableHttpCode(int code) { return RETRYABLE_HTTP_CODES.contains(code); } @@ -85,50 +55,15 @@ static boolean isRetryableHttpCode(int code) { * retried. */ static boolean isRetryableIOException(IOException e) { - // TLS certificate / handshake failures are not retryable. if (e instanceof SSLHandshakeException) return false; String msg = e.getMessage(); - // Protocol mismatch is not retryable. if (msg != null && msg.contains("server gave HTTP response to HTTPS client")) return false; return true; } - /** - * Returns true if the throwable represents a retryable failure. Handles IOException, - * ErrorResponseException, ServerException, and InvalidResponseException. - */ - static boolean isRetryable(Throwable t) { - if (t instanceof CompletionException) t = t.getCause(); - if (t == null) return false; - - if (t instanceof IOException) { - return isRetryableIOException((IOException) t); - } - - if (t instanceof ErrorResponseException) { - ErrorResponseException e = (ErrorResponseException) t; - String code = e.errorResponse().code(); - // "RetryHead" is handled separately by executeHeadAsync — must not be swallowed here. - if ("RetryHead".equals(code)) return false; - if (isRetryableS3Code(code)) return true; - if (e.response() != null && isRetryableHttpCode(e.response().code())) return true; - return false; - } - - if (t instanceof ServerException) { - return isRetryableHttpCode(((ServerException) t).statusCode()); - } - - if (t instanceof InvalidResponseException) { - return isRetryableHttpCode(((InvalidResponseException) t).responseCode()); - } - - return false; - } - /** * Computes the full-jitter exponential backoff delay for retry {@code attempt} (1-indexed: 1 = - * first retry). Matches minio-go's {@code exponentialBackoffWait(i)}: + * first retry). * *
    * attempt=1 → [0, 200 ms]
@@ -141,7 +76,6 @@ static boolean isRetryable(Throwable t) {
    */
   static long computeBackoffMs(int attempt, Random random) {
     if (attempt <= 0) return 0L;
-    // exp = attempt-1 so that attempt=1 maps to base*2^0=200ms cap
     int exp = Math.min(attempt - 1, 30);
     long cap = Math.min(RETRY_CAP_MS, RETRY_BASE_MS * (1L << exp));
     return (long) (random.nextDouble() * cap);
diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java
index dcfa9e9c9..debbf1d84 100644
--- a/api/src/test/java/io/minio/RetryTest.java
+++ b/api/src/test/java/io/minio/RetryTest.java
@@ -18,15 +18,12 @@
 
 import io.minio.errors.ErrorResponseException;
 import io.minio.errors.InvalidResponseException;
-import io.minio.errors.ServerException;
-import io.minio.messages.ErrorResponse;
 import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.net.SocketException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Random;
-import java.util.concurrent.CompletionException;
 import javax.net.ssl.SSLHandshakeException;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
@@ -38,43 +35,6 @@
 /** Unit and integration tests for the automatic retry mechanism. */
 public class RetryTest {
 
-  // ---------------------------------------------------------------------------
-  // Retry.isRetryableS3Code tests
-  // ---------------------------------------------------------------------------
-
-  @Test
-  public void testIsRetryableS3Code_retryable() {
-    Assert.assertTrue(Retry.isRetryableS3Code("RequestError"));
-    Assert.assertTrue(Retry.isRetryableS3Code("RequestTimeout"));
-    Assert.assertTrue(Retry.isRetryableS3Code("Throttling"));
-    Assert.assertTrue(Retry.isRetryableS3Code("ThrottlingException"));
-    Assert.assertTrue(Retry.isRetryableS3Code("RequestLimitExceeded"));
-    Assert.assertTrue(Retry.isRetryableS3Code("RequestThrottled"));
-    Assert.assertTrue(Retry.isRetryableS3Code("InternalError"));
-    Assert.assertTrue(Retry.isRetryableS3Code("ExpiredToken"));
-    Assert.assertTrue(Retry.isRetryableS3Code("ExpiredTokenException"));
-    Assert.assertTrue(Retry.isRetryableS3Code("SlowDown"));
-    Assert.assertTrue(Retry.isRetryableS3Code("SlowDownWrite"));
-    Assert.assertTrue(Retry.isRetryableS3Code("SlowDownRead"));
-  }
-
-  @Test
-  public void testIsRetryableS3Code_notRetryable() {
-    Assert.assertFalse(Retry.isRetryableS3Code("NoSuchKey"));
-    Assert.assertFalse(Retry.isRetryableS3Code("NoSuchBucket"));
-    Assert.assertFalse(Retry.isRetryableS3Code("AccessDenied"));
-    Assert.assertFalse(Retry.isRetryableS3Code("InvalidBucketName"));
-    Assert.assertFalse(Retry.isRetryableS3Code("RetryHead"));
-    Assert.assertFalse(Retry.isRetryableS3Code("MethodNotAllowed"));
-    Assert.assertFalse(Retry.isRetryableS3Code("PreconditionFailed"));
-    Assert.assertFalse(Retry.isRetryableS3Code(""));
-  }
-
-  @Test
-  public void testIsRetryableS3Code_null() {
-    Assert.assertFalse(Retry.isRetryableS3Code(null));
-  }
-
   // ---------------------------------------------------------------------------
   // Retry.isRetryableHttpCode tests
   // ---------------------------------------------------------------------------
@@ -127,64 +87,6 @@ public void testIsRetryableIOException_protocolMismatchNotRetryable() {
         Retry.isRetryableIOException(new IOException("server gave HTTP response to HTTPS client")));
   }
 
-  // ---------------------------------------------------------------------------
-  // Retry.isRetryable(Throwable) tests
-  // ---------------------------------------------------------------------------
-
-  @Test
-  public void testIsRetryable_ioException() {
-    Assert.assertTrue(Retry.isRetryable(new IOException("connection reset")));
-  }
-
-  @Test
-  public void testIsRetryable_ioExceptionWrappedInCompletion() {
-    Assert.assertTrue(
-        Retry.isRetryable(new CompletionException(new IOException("connection reset"))));
-  }
-
-  @Test
-  public void testIsRetryable_sslHandshakeNotRetryable() {
-    Assert.assertFalse(Retry.isRetryable(new SSLHandshakeException("bad cert")));
-  }
-
-  @Test
-  public void testIsRetryable_serverException_retryable() throws Exception {
-    Assert.assertTrue(Retry.isRetryable(new ServerException("Server Error", 500, "")));
-    Assert.assertTrue(Retry.isRetryable(new ServerException("Bad Gateway", 502, "")));
-    Assert.assertTrue(Retry.isRetryable(new ServerException("Service Unavailable", 503, "")));
-  }
-
-  @Test
-  public void testIsRetryable_serverException_notRetryable() throws Exception {
-    Assert.assertFalse(Retry.isRetryable(new ServerException("Not Modified", 304, "")));
-  }
-
-  @Test
-  public void testIsRetryable_invalidResponseException_retryable() throws Exception {
-    Assert.assertTrue(Retry.isRetryable(new InvalidResponseException(500, "text/html", "", "")));
-    Assert.assertTrue(Retry.isRetryable(new InvalidResponseException(503, "text/html", "", "")));
-  }
-
-  @Test
-  public void testIsRetryable_invalidResponseException_notRetryable() throws Exception {
-    Assert.assertFalse(Retry.isRetryable(new InvalidResponseException(403, "text/html", "", "")));
-    Assert.assertFalse(Retry.isRetryable(new InvalidResponseException(404, "text/html", "", "")));
-  }
-
-  @Test
-  public void testIsRetryable_null() {
-    Assert.assertFalse(Retry.isRetryable(null));
-  }
-
-  @Test
-  public void testIsRetryable_retryHeadNotRetryable() throws Exception {
-    // The explicit guard in isRetryable() must fire even if "RetryHead" were in the codes set.
-    ErrorResponse errorResponse =
-        new ErrorResponse("RetryHead", null, null, null, null, null, null);
-    ErrorResponseException e = new ErrorResponseException(errorResponse, null, "");
-    Assert.assertFalse(Retry.isRetryable(e));
-  }
-
   // ---------------------------------------------------------------------------
   // Retry.computeBackoffMs tests
   // ---------------------------------------------------------------------------
@@ -253,18 +155,6 @@ public void testComputeBackoffMs_highAttemptCapped() {
           + "testtest"
           + "";
 
-  private static final String INTERNAL_ERROR_XML =
-      ""
-          + "InternalError"
-          + "We encountered an internal error. Please try again."
-          + "/abc";
-
-  private static final String THROTTLE_XML =
-      ""
-          + "SlowDown"
-          + "Please reduce your request rate."
-          + "/abc";
-
   /** Returns a 200 OK response with valid ListAllMyBucketsResult body. */
   private MockResponse successResponse() {
     return new MockResponse()
@@ -289,26 +179,10 @@ private MockResponse serviceUnavailable503() {
         .setBody(new Buffer().writeUtf8("Service Unavailable"));
   }
 
-  /** Returns a 500 S3 InternalError XML response. */
-  private MockResponse s3InternalErrorResponse() {
-    return new MockResponse()
-        .setResponseCode(500)
-        .setHeader("Content-Type", "application/xml")
-        .setBody(new Buffer().writeUtf8(INTERNAL_ERROR_XML));
-  }
-
-  /** Returns a 429 TooManyRequests with S3 SlowDown XML. */
-  private MockResponse s3ThrottleResponse() {
-    return new MockResponse()
-        .setResponseCode(429)
-        .setHeader("Content-Type", "application/xml")
-        .setBody(new Buffer().writeUtf8(THROTTLE_XML));
-  }
-
   @Test
   public void testRetryOn500HtmlThenSuccess() throws Exception {
     try (MockWebServer server = new MockWebServer()) {
-      // First request → 500 HTML (non-XML) → InvalidResponseException(500) → retryable
+      // First request → 500 HTML (non-XML) → retryable
       server.enqueue(serverError500Html());
       // Second request (retry) → 200 OK
       server.enqueue(successResponse());
@@ -338,36 +212,6 @@ public void testRetryOn503ThenSuccess() throws Exception {
     }
   }
 
-  @Test
-  public void testRetryOnS3InternalErrorThenSuccess() throws Exception {
-    try (MockWebServer server = new MockWebServer()) {
-      server.enqueue(s3InternalErrorResponse());
-      server.enqueue(successResponse());
-      server.start();
-
-      MinioClient client =
-          MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build();
-      client.listBuckets();
-
-      Assert.assertEquals(2, server.getRequestCount());
-    }
-  }
-
-  @Test
-  public void testRetryOnS3ThrottleThenSuccess() throws Exception {
-    try (MockWebServer server = new MockWebServer()) {
-      server.enqueue(s3ThrottleResponse());
-      server.enqueue(successResponse());
-      server.start();
-
-      MinioClient client =
-          MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build();
-      client.listBuckets();
-
-      Assert.assertEquals(2, server.getRequestCount());
-    }
-  }
-
   @Test
   public void testNoRetryOn404() throws Exception {
     String notFoundXml =

From 1997b48168f7f94ea486d07cb378ba5ef375bbbc Mon Sep 17 00:00:00 2001
From: Allan Roger Reid 
Date: Thu, 7 May 2026 16:37:10 +0000
Subject: [PATCH 09/15] fix: narrow throws clauses in RetryTest to satisfy
 SpotBugs

Replace `throws Exception` with the actually-thrown checked exceptions
(`IOException, MinioException`, plus `InterruptedException` for tests
calling `MockWebServer.takeRequest()`). Drops `throws` entirely on
`testSetMaxRetriesValidation` since it throws only an unchecked
`IllegalArgumentException`.

Fixes spotbugsTest THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
findings on Java 8/11/17/21/25.
---
 api/src/test/java/io/minio/RetryTest.java | 35 ++++++++++++-----------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java
index debbf1d84..ab0c78ec7 100644
--- a/api/src/test/java/io/minio/RetryTest.java
+++ b/api/src/test/java/io/minio/RetryTest.java
@@ -18,6 +18,7 @@
 
 import io.minio.errors.ErrorResponseException;
 import io.minio.errors.InvalidResponseException;
+import io.minio.errors.MinioException;
 import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.net.SocketException;
@@ -180,7 +181,7 @@ private MockResponse serviceUnavailable503() {
   }
 
   @Test
-  public void testRetryOn500HtmlThenSuccess() throws Exception {
+  public void testRetryOn500HtmlThenSuccess() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       // First request → 500 HTML (non-XML) → retryable
       server.enqueue(serverError500Html());
@@ -198,7 +199,7 @@ public void testRetryOn500HtmlThenSuccess() throws Exception {
   }
 
   @Test
-  public void testRetryOn503ThenSuccess() throws Exception {
+  public void testRetryOn503ThenSuccess() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(serviceUnavailable503());
       server.enqueue(successResponse());
@@ -213,7 +214,7 @@ public void testRetryOn503ThenSuccess() throws Exception {
   }
 
   @Test
-  public void testNoRetryOn404() throws Exception {
+  public void testNoRetryOn404() throws IOException, MinioException {
     String notFoundXml =
         ""
             + "NoSuchBucketnot found"
@@ -242,7 +243,7 @@ public void testNoRetryOn404() throws Exception {
   }
 
   @Test
-  public void testNoRetryOn403() throws Exception {
+  public void testNoRetryOn403() throws IOException, MinioException {
     String accessDeniedXml =
         ""
             + "AccessDeniedAccess Denied"
@@ -270,7 +271,7 @@ public void testNoRetryOn403() throws Exception {
   }
 
   @Test
-  public void testRetryExhaustedThrowsLastError() throws Exception {
+  public void testRetryExhaustedThrowsLastError() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       // All 3 attempts fail with 500
       server.enqueue(serverError500Html());
@@ -291,7 +292,7 @@ public void testRetryExhaustedThrowsLastError() throws Exception {
   }
 
   @Test
-  public void testMaxRetriesOneDisablesRetry() throws Exception {
+  public void testMaxRetriesOneDisablesRetry() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(serverError500Html());
       // Second response should never be reached
@@ -312,7 +313,7 @@ public void testMaxRetriesOneDisablesRetry() throws Exception {
   }
 
   @Test
-  public void testSetMaxRetriesPostConstruction() throws Exception {
+  public void testSetMaxRetriesPostConstruction() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       // Two failures enqueued, but maxRetries will be set to 1 after construction
       server.enqueue(serverError500Html());
@@ -333,7 +334,7 @@ public void testSetMaxRetriesPostConstruction() throws Exception {
   }
 
   @Test
-  public void testMultipleRetrySucceedsOnThirdAttempt() throws Exception {
+  public void testMultipleRetrySucceedsOnThirdAttempt() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(serverError500Html());
       server.enqueue(serviceUnavailable503());
@@ -349,7 +350,7 @@ public void testMultipleRetrySucceedsOnThirdAttempt() throws Exception {
   }
 
   @Test
-  public void testRetry429ThenSuccess() throws Exception {
+  public void testRetry429ThenSuccess() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(
           new MockResponse()
@@ -368,7 +369,7 @@ public void testRetry429ThenSuccess() throws Exception {
   }
 
   @Test
-  public void testRetry502ThenSuccess() throws Exception {
+  public void testRetry502ThenSuccess() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(
           new MockResponse()
@@ -387,7 +388,7 @@ public void testRetry502ThenSuccess() throws Exception {
   }
 
   @Test
-  public void testRetry504ThenSuccess() throws Exception {
+  public void testRetry504ThenSuccess() throws IOException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(
           new MockResponse()
@@ -411,13 +412,13 @@ public void testMaxRetriesBuilderValidation() {
   }
 
   @Test(expected = IllegalArgumentException.class)
-  public void testSetMaxRetriesValidation() throws Exception {
+  public void testSetMaxRetriesValidation() {
     MinioClient client = MinioClient.builder().endpoint("http://localhost:9000").build();
     client.setMaxRetries(0);
   }
 
   @Test
-  public void testDefaultMaxRetriesIsConfigurable() throws Exception {
+  public void testDefaultMaxRetriesIsConfigurable() throws IOException, MinioException {
     // Use maxRetries=4 to keep test fast (delays ≤ 200+400+800ms ≈ 1.4s max)
     int retries = 4;
     try (MockWebServer server = new MockWebServer()) {
@@ -436,7 +437,7 @@ public void testDefaultMaxRetriesIsConfigurable() throws Exception {
   }
 
   @Test
-  public void testRequestBodyRetried() throws Exception {
+  public void testRequestBodyRetried() throws IOException, MinioException {
     // Byte array body (seekable) should be retried
     try (MockWebServer server = new MockWebServer()) {
       // First putObject fails, second succeeds with ETag
@@ -469,7 +470,8 @@ public void testRequestBodyRetried() throws Exception {
   }
 
   @Test
-  public void testRandomAccessFileBodyRetried() throws Exception {
+  public void testRandomAccessFileBodyRetried()
+      throws IOException, InterruptedException, MinioException {
     Path tmpFile = Files.createTempFile("retry-raf-test", ".bin");
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(serverError500Html());
@@ -511,7 +513,8 @@ public void testRandomAccessFileBodyRetried() throws Exception {
   }
 
   @Test
-  public void testRecordedRequestHeaders() throws Exception {
+  public void testRecordedRequestHeaders()
+      throws IOException, InterruptedException, MinioException {
     try (MockWebServer server = new MockWebServer()) {
       server.enqueue(serverError500Html());
       server.enqueue(successResponse());

From ba653128e9194fcacd8be666d01b30ca06e1edba Mon Sep 17 00:00:00 2001
From: Allan Roger Reid 
Date: Thu, 7 May 2026 18:29:28 +0000
Subject: [PATCH 10/15] refactor: strip retry to balamurugana's interceptor
 proposal scope
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per @balamurugana's review on PR #1701, replace the
CompletableFuture/scheduler retry orchestration with a self-contained
OkHttp interceptor that retries on retryable HTTP status codes with
exponential backoff and full jitter.

- Http.RetryInterceptor: hard-coded MAX_RETRIES=5, BASE_DELAY_MS=200,
  MAX_DELAY_MS=30s, RETRYABLE_STATUS_CODES = 408/429/499/500/502/503/
  504/520. Bit-shift exponent capped to avoid overflow at high attempt
  counts; jitter bound guarded with Math.max(1, cap).
- Wired into Http.newDefaultClient() with a single addInterceptor call.
- Drop public maxRetries API on MinioClient/MinioAsyncClient/BaseS3Client
  and Builder.maxRetries; the interceptor's params are constants.
- Drop Retry.java entirely — RETRYABLE_HTTP_CODES inlined; the S3-error-
  code retry set, isRetryable(Throwable), isRetryableIOException, and
  computeBackoffMs helpers are no longer needed.
- IOException retry now relies on OkHttp's default
  retryOnConnectionFailure(true); only the interceptor handles status
  codes. No double-retry, no overlap.
- Drop Http.RequestBody.isReplayable() and the body-replayability gate;
  the raw okhttp3 RequestBody body path is unused in production code,
  and byte[]/ByteBuffer/RandomAccessFile bodies replay safely via
  writeTo().
- Drop InvalidResponseException.responseCode() accessor (added solely
  for the deleted retry classifier).
- RetryTest: trim from 539 lines to two integration smoke tests
  (retry on 503->200, no retry on 404).

Validated locally:
- :api:check (unit + spotbugs + spotless) green on Java 17
- :functional:runFunctionalTest passes through CORS test (modulo a
  pre-existing CORSConfiguration.equals() bug unrelated to this PR)
- mint suite (overlayed JARs in mint container, erasure mode):
  "All tests ran successfully", PASS in 42.8s
---
 api/src/main/java/io/minio/BaseS3Client.java  |  36 +-
 api/src/main/java/io/minio/Http.java          |  83 ++--
 .../main/java/io/minio/MinioAsyncClient.java  |  16 +-
 api/src/main/java/io/minio/MinioClient.java   |  18 -
 api/src/main/java/io/minio/Retry.java         |  85 ----
 .../errors/InvalidResponseException.java      |   8 -
 api/src/test/java/io/minio/RetryTest.java     | 468 +-----------------
 7 files changed, 46 insertions(+), 668 deletions(-)
 delete mode 100644 api/src/main/java/io/minio/Retry.java

diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java
index 1d545cbf4..81dff4e04 100644
--- a/api/src/main/java/io/minio/BaseS3Client.java
+++ b/api/src/main/java/io/minio/BaseS3Client.java
@@ -110,36 +110,20 @@ public abstract class BaseS3Client implements AutoCloseable {
   protected Provider provider;
   protected OkHttpClient httpClient;
   protected boolean closeHttpClient;
-  protected volatile int maxRetries = Retry.MAX_RETRY;
 
   protected BaseS3Client(
       Http.BaseUrl baseUrl, Provider provider, OkHttpClient httpClient, boolean closeHttpClient) {
     this.baseUrl = baseUrl;
     this.provider = provider;
+    this.httpClient = httpClient;
     this.closeHttpClient = closeHttpClient;
-    this.httpClient = wrapWithRetry(httpClient);
   }
 
   protected BaseS3Client(BaseS3Client client) {
     this.baseUrl = client.baseUrl;
     this.provider = client.provider;
+    this.httpClient = client.httpClient;
     this.closeHttpClient = client.closeHttpClient;
-    this.maxRetries = client.maxRetries;
-    this.httpClient = wrapWithRetry(client.httpClient);
-  }
-
-  /**
-   * Adds the retry interceptor to {@code client}. If {@code client} already has one (e.g. from a
-   * prior wrap via copy constructor), it is replaced so the interceptor reads from this instance's
-   * mutable {@code maxRetries} field.
-   */
-  private OkHttpClient wrapWithRetry(OkHttpClient client) {
-    OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false);
-    builder.interceptors().removeIf(i -> i instanceof Http.RetryInterceptor);
-    return builder
-        .addInterceptor(
-            new Http.RetryInterceptor(() -> this.maxRetries, Retry.RETRYABLE_HTTP_CODES))
-        .build();
   }
 
   /** Closes underneath HTTP client. */
@@ -151,18 +135,6 @@ public void close() {
     }
   }
 
-  /**
-   * Sets the maximum number of retry attempts for failed S3 requests. Requests with non-seekable
-   * bodies are never retried regardless of this value. The default is {@code Retry.MAX_RETRY} (10).
-   * Pass 1 to disable automatic retries.
-   *
-   * @param maxRetries maximum attempts (must be >= 1).
-   */
-  public void setMaxRetries(int maxRetries) {
-    if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1");
-    this.maxRetries = maxRetries;
-  }
-
   /**
    * Sets HTTP connect, write and read timeouts. A value of 0 means no timeout, otherwise values
    * must be between 1 and Integer.MAX_VALUE when converted to milliseconds.
@@ -295,8 +267,8 @@ private String[] handleRedirectResponse(
   }
 
   /**
-   * Execute HTTP request asynchronously for given parameters. Network-level retry (transient HTTP
-   * status codes and IOExceptions) is handled by {@link Http.RetryInterceptor} on the client.
+   * Execute HTTP request asynchronously for given parameters. Retries on retryable HTTP status
+   * codes are handled by {@link Http.RetryInterceptor} installed on the default HTTP client.
    */
   protected CompletableFuture executeAsync(Http.S3Request s3request, String region) {
     Credentials credentials = (provider == null) ? null : provider.fetch();
diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java
index 77619e025..4c1a65091 100644
--- a/api/src/main/java/io/minio/Http.java
+++ b/api/src/main/java/io/minio/Http.java
@@ -54,7 +54,6 @@
 import java.util.Set;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
-import java.util.function.IntSupplier;
 import java.util.regex.Matcher;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -623,6 +622,7 @@ public static OkHttpClient newDefaultClient() {
             .writeTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS)
             .readTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS)
             .protocols(Arrays.asList(Protocol.HTTP_1_1))
+            .addInterceptor(new RetryInterceptor())
             .build();
     try {
       return enableExternalCertificatesFromEnv(client);
@@ -687,58 +687,47 @@ public static OkHttpClient setTimeout(
   }
 
   /**
-   * OkHttp interceptor that retries requests on retryable HTTP status codes and IOExceptions with
-   * exponential backoff and full jitter. Requests with non-replayable bodies are sent once.
+   * OkHttp interceptor that retries requests on retryable HTTP status codes with exponential
+   * backoff and full jitter.
    */
   public static class RetryInterceptor implements Interceptor {
-    private final IntSupplier maxRetriesSupplier;
-    private final Set retryStatusCodes;
-
-    public RetryInterceptor(IntSupplier maxRetriesSupplier, Set retryStatusCodes) {
-      this.maxRetriesSupplier = Objects.requireNonNull(maxRetriesSupplier, "maxRetriesSupplier");
-      this.retryStatusCodes = Objects.requireNonNull(retryStatusCodes, "retryStatusCodes");
-    }
-
-    private static boolean isReplayable(okhttp3.RequestBody body) {
-      if (body == null) return true;
-      if (body instanceof RequestBody) return ((RequestBody) body).isReplayable();
-      return false;
-    }
+    private static final int MAX_RETRIES = 5;
+    private static final long BASE_DELAY_MS = 200L;
+    private static final long MAX_DELAY_MS = 30_000L;
+    private static final Set RETRYABLE_STATUS_CODES =
+        ImmutableSet.of(
+            408, // Request Timeout
+            429, // Too Many Requests
+            499, // Client Closed Request (nginx)
+            500, // Internal Server Error
+            502, // Bad Gateway
+            503, // Service Unavailable
+            504, // Gateway Timeout
+            520); // Cloudflare unknown error
 
     @Override
     public okhttp3.Response intercept(Chain chain) throws IOException {
       okhttp3.Request request = chain.request();
-      int maxAttempts =
-          isReplayable(request.body()) ? Math.max(1, maxRetriesSupplier.getAsInt()) : 1;
-
-      okhttp3.Response response = null;
-      IOException lastException = null;
-      for (int attempt = 0; attempt < maxAttempts; attempt++) {
-        if (attempt > 0) {
-          long delayMs = Retry.computeBackoffMs(attempt, ThreadLocalRandom.current());
-          if (delayMs > 0) {
-            try {
-              Thread.sleep(delayMs);
-            } catch (InterruptedException ie) {
-              Thread.currentThread().interrupt();
-              throw new IOException("retry interrupted", ie);
-            }
-          }
-          if (response != null) response.close();
-        }
+      okhttp3.Response response = chain.proceed(request);
 
+      int tryCount = 0;
+      while (RETRYABLE_STATUS_CODES.contains(response.code()) && tryCount < MAX_RETRIES) {
+        tryCount++;
+        response.close();
+
+        // Cap exponent to avoid bit-shift overflow at high attempt counts.
+        int exp = Math.min(tryCount, 30);
+        long backoffCap = Math.min(MAX_DELAY_MS, BASE_DELAY_MS * (1L << exp));
+        long jittered = ThreadLocalRandom.current().nextLong(0, Math.max(1, backoffCap));
         try {
-          response = chain.proceed(request);
-          if (!retryStatusCodes.contains(response.code())) return response;
-          lastException = null;
-        } catch (IOException e) {
-          if (!Retry.isRetryableIOException(e)) throw e;
-          lastException = e;
-          response = null;
+          Thread.sleep(jittered);
+        } catch (InterruptedException ie) {
+          Thread.currentThread().interrupt();
+          throw new IOException("retry interrupted", ie);
         }
-      }
 
-      if (lastException != null) throw lastException;
+        response = chain.proceed(request);
+      }
       return response;
     }
   }
@@ -950,14 +939,6 @@ public void writeTo(BufferedSink sink) throws IOException {
         sink.write(bytes, 0, (int) length);
       }
     }
-
-    /**
-     * Returns true if {@link #writeTo} can be invoked multiple times. False for raw okhttp3
-     * RequestBody since it may consume a one-shot source.
-     */
-    public boolean isReplayable() {
-      return body == null;
-    }
   }
 
   /** HTTP methods. */
diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java
index f0bdab3c4..766ce9fa8 100644
--- a/api/src/main/java/io/minio/MinioAsyncClient.java
+++ b/api/src/main/java/io/minio/MinioAsyncClient.java
@@ -157,7 +157,6 @@ public static final class Builder {
     private Provider provider;
     private OkHttpClient httpClient;
     private boolean closeHttpClient;
-    private int maxRetries = Retry.MAX_RETRY;
 
     public Builder baseUrl(Http.BaseUrl baseUrl) {
       if (baseUrl.region() == null) {
@@ -219,16 +218,6 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) {
       return this;
     }
 
-    /**
-     * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries.
-     * Defaults to {@code 10}.
-     */
-    public Builder maxRetries(int maxRetries) {
-      if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1");
-      this.maxRetries = maxRetries;
-      return this;
-    }
-
     public MinioAsyncClient build() {
       Utils.validateNotNull(baseUrl, "endpoint");
 
@@ -244,10 +233,7 @@ public MinioAsyncClient build() {
         httpClient = Http.newDefaultClient();
       }
 
-      MinioAsyncClient client =
-          new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient);
-      client.maxRetries = maxRetries;
-      return client;
+      return new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient);
     }
   }
 
diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java
index 098f21780..03dd587a9 100644
--- a/api/src/main/java/io/minio/MinioClient.java
+++ b/api/src/main/java/io/minio/MinioClient.java
@@ -1976,15 +1976,6 @@ public void setAwsS3Prefix(String awsS3Prefix) {
     asyncClient.setAwsS3Prefix(awsS3Prefix);
   }
 
-  /**
-   * Sets the maximum number of retry attempts. Pass 1 to disable automatic retries.
-   *
-   * @param maxRetries maximum attempts (must be >= 1).
-   */
-  public void setMaxRetries(int maxRetries) {
-    asyncClient.setMaxRetries(maxRetries);
-  }
-
   /** Closes underneath async client. */
   @Override
   public void close() throws Exception {
@@ -2049,15 +2040,6 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) {
       return this;
     }
 
-    /**
-     * Sets the maximum number of retry attempts per request. Pass 1 to disable automatic retries.
-     * Defaults to {@code 10}.
-     */
-    public Builder maxRetries(int maxRetries) {
-      asyncClientBuilder.maxRetries(maxRetries);
-      return this;
-    }
-
     public MinioClient build() {
       MinioAsyncClient asyncClient = asyncClientBuilder.build();
       return new MinioClient(asyncClient);
diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java
deleted file mode 100644
index f67ba10f7..000000000
--- a/api/src/main/java/io/minio/Retry.java
+++ /dev/null
@@ -1,85 +0,0 @@
-/*
- * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2026 MinIO, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package io.minio;
-
-import com.google.common.collect.ImmutableSet;
-import java.io.IOException;
-import java.util.Random;
-import java.util.Set;
-import javax.net.ssl.SSLHandshakeException;
-
-/** Retry configuration and helpers for S3 request execution. */
-class Retry {
-  /** Default maximum number of retry attempts per request. */
-  static final int MAX_RETRY = 10;
-
-  /** Base sleep unit for exponential backoff (milliseconds). */
-  static final long RETRY_BASE_MS = 200L;
-
-  /** Maximum sleep cap for exponential backoff (milliseconds). */
-  static final long RETRY_CAP_MS = 1_000L;
-
-  /** HTTP status codes that should trigger a retry. */
-  static final Set RETRYABLE_HTTP_CODES =
-      ImmutableSet.of(
-          408, // Request Timeout
-          429, // Too Many Requests
-          499, // Client Closed Request (nginx)
-          500, // Internal Server Error
-          502, // Bad Gateway
-          503, // Service Unavailable
-          504, // Gateway Timeout
-          520); // Cloudflare unknown error
-
-  static boolean isRetryableHttpCode(int code) {
-    return RETRYABLE_HTTP_CODES.contains(code);
-  }
-
-  /**
-   * Returns true if the IOException is retryable. Non-retryable: TLS handshake failures, HTTP/HTTPS
-   * protocol mismatch. Everything else (connection reset, EOF, server closed idle connection) is
-   * retried.
-   */
-  static boolean isRetryableIOException(IOException e) {
-    if (e instanceof SSLHandshakeException) return false;
-    String msg = e.getMessage();
-    if (msg != null && msg.contains("server gave HTTP response to HTTPS client")) return false;
-    return true;
-  }
-
-  /**
-   * Computes the full-jitter exponential backoff delay for retry {@code attempt} (1-indexed: 1 =
-   * first retry).
-   *
-   * 
-   * attempt=1 → [0, 200 ms]
-   * attempt=2 → [0, 400 ms]
-   * attempt=3 → [0, 800 ms]
-   * attempt=4+→ [0, 1000 ms] (capped)
-   * 
- * - * Pass {@code attempt <= 0} to get 0 (no delay). - */ - static long computeBackoffMs(int attempt, Random random) { - if (attempt <= 0) return 0L; - int exp = Math.min(attempt - 1, 30); - long cap = Math.min(RETRY_CAP_MS, RETRY_BASE_MS * (1L << exp)); - return (long) (random.nextDouble() * cap); - } - - private Retry() {} -} diff --git a/api/src/main/java/io/minio/errors/InvalidResponseException.java b/api/src/main/java/io/minio/errors/InvalidResponseException.java index abf3c225a..1305b3cdd 100644 --- a/api/src/main/java/io/minio/errors/InvalidResponseException.java +++ b/api/src/main/java/io/minio/errors/InvalidResponseException.java @@ -20,8 +20,6 @@ public class InvalidResponseException extends MinioException { private static final long serialVersionUID = -4793742105569629274L; - private final int responseCode; - public InvalidResponseException( int responseCode, String contentType, String body, String httpTrace) { super( @@ -32,11 +30,5 @@ public InvalidResponseException( + ", body: " + body, httpTrace); - this.responseCode = responseCode; - } - - /** Returns the HTTP response code that triggered this exception. */ - public int responseCode() { - return responseCode; } } diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index ab0c78ec7..9345ef8bd 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -17,146 +17,22 @@ package io.minio; import io.minio.errors.ErrorResponseException; -import io.minio.errors.InvalidResponseException; import io.minio.errors.MinioException; import java.io.IOException; -import java.io.RandomAccessFile; -import java.net.SocketException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Random; -import javax.net.ssl.SSLHandshakeException; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; import okio.Buffer; import org.junit.Assert; import org.junit.Test; -/** Unit and integration tests for the automatic retry mechanism. */ +/** Integration tests for {@link Http.RetryInterceptor}. */ public class RetryTest { - - // --------------------------------------------------------------------------- - // Retry.isRetryableHttpCode tests - // --------------------------------------------------------------------------- - - @Test - public void testIsRetryableHttpCode_retryable() { - Assert.assertTrue(Retry.isRetryableHttpCode(408)); - Assert.assertTrue(Retry.isRetryableHttpCode(429)); - Assert.assertTrue(Retry.isRetryableHttpCode(499)); - Assert.assertTrue(Retry.isRetryableHttpCode(500)); - Assert.assertTrue(Retry.isRetryableHttpCode(502)); - Assert.assertTrue(Retry.isRetryableHttpCode(503)); - Assert.assertTrue(Retry.isRetryableHttpCode(504)); - Assert.assertTrue(Retry.isRetryableHttpCode(520)); - } - - @Test - public void testIsRetryableHttpCode_notRetryable() { - Assert.assertFalse(Retry.isRetryableHttpCode(200)); - Assert.assertFalse(Retry.isRetryableHttpCode(201)); - Assert.assertFalse(Retry.isRetryableHttpCode(400)); - Assert.assertFalse(Retry.isRetryableHttpCode(403)); - Assert.assertFalse(Retry.isRetryableHttpCode(404)); - Assert.assertFalse(Retry.isRetryableHttpCode(409)); - Assert.assertFalse(Retry.isRetryableHttpCode(412)); - } - - // --------------------------------------------------------------------------- - // Retry.isRetryableIOException tests - // --------------------------------------------------------------------------- - - @Test - public void testIsRetryableIOException_retryable() { - Assert.assertTrue(Retry.isRetryableIOException(new IOException("connection reset"))); - Assert.assertTrue(Retry.isRetryableIOException(new IOException("EOF"))); - Assert.assertTrue( - Retry.isRetryableIOException(new IOException("http: server closed idle connection"))); - Assert.assertTrue(Retry.isRetryableIOException(new SocketException("Connection timed out"))); - Assert.assertTrue(Retry.isRetryableIOException(new java.net.SocketTimeoutException("read"))); - } - - @Test - public void testIsRetryableIOException_sslHandshakeNotRetryable() { - Assert.assertFalse(Retry.isRetryableIOException(new SSLHandshakeException("cert not trusted"))); - } - - @Test - public void testIsRetryableIOException_protocolMismatchNotRetryable() { - Assert.assertFalse( - Retry.isRetryableIOException(new IOException("server gave HTTP response to HTTPS client"))); - } - - // --------------------------------------------------------------------------- - // Retry.computeBackoffMs tests - // --------------------------------------------------------------------------- - - @Test - public void testComputeBackoffMs_attemptZeroIsZero() { - Random random = new Random(42); - Assert.assertEquals(0L, Retry.computeBackoffMs(0, random)); - } - - @Test - public void testComputeBackoffMs_negativeAttemptIsZero() { - Random random = new Random(42); - Assert.assertEquals(0L, Retry.computeBackoffMs(-1, random)); - } - - @Test - public void testComputeBackoffMs_firstRetryWithinCap() { - // attempt=1 (first retry after one failure): cap = min(1000, 200*2^0) = 200ms - for (int seed = 0; seed < 20; seed++) { - Random random = new Random(seed); - long delay = Retry.computeBackoffMs(1, random); - Assert.assertTrue("first retry delay must be in [0, 200ms]", delay >= 0 && delay <= 200); - } - } - - @Test - public void testComputeBackoffMs_secondRetryWithinCap() { - // attempt=2 (second retry): cap = min(1000, 200*2^1) = 400ms - for (int seed = 0; seed < 20; seed++) { - Random random = new Random(seed); - long delay = Retry.computeBackoffMs(2, random); - Assert.assertTrue("second retry delay must be in [0, 400ms]", delay >= 0 && delay <= 400); - } - } - - @Test - public void testComputeBackoffMs_cappedAtRetryCapMs() { - // With attempt=5, uncapped = 200 * 32 = 6400ms, but should be capped at 1000ms - for (int seed = 0; seed < 20; seed++) { - Random random = new Random(seed); - long delay = Retry.computeBackoffMs(5, random); - Assert.assertTrue( - "delay must be <= RETRY_CAP_MS (" + Retry.RETRY_CAP_MS + ")", - delay <= Retry.RETRY_CAP_MS); - Assert.assertTrue("delay must be >= 0", delay >= 0); - } - } - - @Test - public void testComputeBackoffMs_highAttemptCapped() { - for (int seed = 0; seed < 20; seed++) { - Random random = new Random(seed); - long delay = Retry.computeBackoffMs(100, random); - Assert.assertTrue("delay must be <= RETRY_CAP_MS", delay <= Retry.RETRY_CAP_MS); - } - } - - // --------------------------------------------------------------------------- - // Integration tests via MockWebServer - // --------------------------------------------------------------------------- - private static final String LIST_BUCKETS_OK = "" + "" + "testtest" + ""; - /** Returns a 200 OK response with valid ListAllMyBucketsResult body. */ private MockResponse successResponse() { return new MockResponse() .setResponseCode(200) @@ -164,16 +40,7 @@ private MockResponse successResponse() { .setBody(new Buffer().writeUtf8(LIST_BUCKETS_OK)); } - /** Returns a 500 server error response with HTML body (non-XML). */ - private MockResponse serverError500Html() { - return new MockResponse() - .setResponseCode(500) - .setHeader("Content-Type", "text/html") - .setBody(new Buffer().writeUtf8("Internal Server Error")); - } - - /** Returns a 503 error with HTML body. */ - private MockResponse serviceUnavailable503() { + private MockResponse retryableServerError() { return new MockResponse() .setResponseCode(503) .setHeader("Content-Type", "text/html") @@ -181,32 +48,13 @@ private MockResponse serviceUnavailable503() { } @Test - public void testRetryOn500HtmlThenSuccess() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - // First request → 500 HTML (non-XML) → retryable - server.enqueue(serverError500Html()); - // Second request (retry) → 200 OK - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - // Should succeed after one retry - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); - } - } - - @Test - public void testRetryOn503ThenSuccess() throws IOException, MinioException { + public void testRetryOnRetryableStatusThenSuccess() throws IOException, MinioException { try (MockWebServer server = new MockWebServer()) { - server.enqueue(serviceUnavailable503()); + server.enqueue(retryableServerError()); server.enqueue(successResponse()); server.start(); - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).build(); client.listBuckets(); Assert.assertEquals(2, server.getRequestCount()); @@ -214,11 +62,11 @@ public void testRetryOn503ThenSuccess() throws IOException, MinioException { } @Test - public void testNoRetryOn404() throws IOException, MinioException { + public void testNoRetryOn4xx() throws IOException, MinioException { String notFoundXml = "" + "NoSuchBucketnot found" - + "/testabc"; + + "/abc
"; try (MockWebServer server = new MockWebServer()) { server.enqueue( @@ -228,312 +76,14 @@ public void testNoRetryOn404() throws IOException, MinioException { .setBody(new Buffer().writeUtf8(notFoundXml))); server.start(); - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).build(); try { client.listBuckets(); - Assert.fail("expected exception"); + Assert.fail("expected ErrorResponseException"); } catch (ErrorResponseException e) { Assert.assertEquals("NoSuchBucket", e.errorResponse().code()); - Assert.assertEquals(404, e.response().code()); - } - // Must not retry — only 1 request should have been made - Assert.assertEquals(1, server.getRequestCount()); - } - } - - @Test - public void testNoRetryOn403() throws IOException, MinioException { - String accessDeniedXml = - "" - + "AccessDeniedAccess Denied" - + "/abc"; - - try (MockWebServer server = new MockWebServer()) { - server.enqueue( - new MockResponse() - .setResponseCode(403) - .setHeader("Content-Type", "application/xml") - .setBody(new Buffer().writeUtf8(accessDeniedXml))); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); - try { - client.listBuckets(); - Assert.fail("expected exception"); - } catch (ErrorResponseException e) { - Assert.assertEquals("AccessDenied", e.errorResponse().code()); - Assert.assertEquals(403, e.response().code()); - } - Assert.assertEquals(1, server.getRequestCount()); - } - } - - @Test - public void testRetryExhaustedThrowsLastError() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - // All 3 attempts fail with 500 - server.enqueue(serverError500Html()); - server.enqueue(serverError500Html()); - server.enqueue(serverError500Html()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); - try { - client.listBuckets(); - Assert.fail("expected exception after exhausted retries"); - } catch (InvalidResponseException e) { - Assert.assertEquals(500, e.responseCode()); - } - Assert.assertEquals(3, server.getRequestCount()); - } - } - - @Test - public void testMaxRetriesOneDisablesRetry() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue(serverError500Html()); - // Second response should never be reached - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(1).build(); - try { - client.listBuckets(); - Assert.fail("expected exception"); - } catch (InvalidResponseException e) { - Assert.assertEquals(500, e.responseCode()); - } - // maxRetries=1 means a single attempt; must not retry - Assert.assertEquals(1, server.getRequestCount()); - } - } - - @Test - public void testSetMaxRetriesPostConstruction() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - // Two failures enqueued, but maxRetries will be set to 1 after construction - server.enqueue(serverError500Html()); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.setMaxRetries(1); // override to disable retries - try { - client.listBuckets(); - Assert.fail("expected exception"); - } catch (InvalidResponseException e) { - Assert.assertEquals(500, e.responseCode()); } Assert.assertEquals(1, server.getRequestCount()); } } - - @Test - public void testMultipleRetrySucceedsOnThirdAttempt() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue(serverError500Html()); - server.enqueue(serviceUnavailable503()); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); - client.listBuckets(); - - Assert.assertEquals(3, server.getRequestCount()); - } - } - - @Test - public void testRetry429ThenSuccess() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue( - new MockResponse() - .setResponseCode(429) - .setHeader("Content-Type", "text/html") - .setBody(new Buffer().writeUtf8("Too Many Requests"))); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); - } - } - - @Test - public void testRetry502ThenSuccess() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue( - new MockResponse() - .setResponseCode(502) - .setHeader("Content-Type", "text/html") - .setBody(new Buffer().writeUtf8("Bad Gateway"))); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); - } - } - - @Test - public void testRetry504ThenSuccess() throws IOException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue( - new MockResponse() - .setResponseCode(504) - .setHeader("Content-Type", "text/html") - .setBody(new Buffer().writeUtf8("Gateway Timeout"))); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); - } - } - - @Test(expected = IllegalArgumentException.class) - public void testMaxRetriesBuilderValidation() { - MinioClient.builder().endpoint("http://localhost:9000").maxRetries(0).build(); - } - - @Test(expected = IllegalArgumentException.class) - public void testSetMaxRetriesValidation() { - MinioClient client = MinioClient.builder().endpoint("http://localhost:9000").build(); - client.setMaxRetries(0); - } - - @Test - public void testDefaultMaxRetriesIsConfigurable() throws IOException, MinioException { - // Use maxRetries=4 to keep test fast (delays ≤ 200+400+800ms ≈ 1.4s max) - int retries = 4; - try (MockWebServer server = new MockWebServer()) { - for (int i = 0; i < retries - 1; i++) { - server.enqueue(serverError500Html()); - } - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(retries).build(); - client.listBuckets(); - - Assert.assertEquals(retries, server.getRequestCount()); - } - } - - @Test - public void testRequestBodyRetried() throws IOException, MinioException { - // Byte array body (seekable) should be retried - try (MockWebServer server = new MockWebServer()) { - // First putObject fails, second succeeds with ETag - server.enqueue(serverError500Html()); - server.enqueue( - new MockResponse() - .setResponseCode(200) - .setHeader("ETag", "\"abc123\"") - .setHeader("Content-Length", "0")); - server.start(); - - // Specify region so the SDK doesn't need a separate GetBucketLocation request - MinioClient client = - MinioClient.builder() - .endpoint(server.url("").toString()) - .credentials("access", "secret") - .region("us-east-1") - .maxRetries(2) - .build(); - - client.putObject( - PutObjectArgs.builder() - .bucket("test-bucket") - .object("test-object") - .data(new byte[0], 0) - .build()); - - Assert.assertEquals(2, server.getRequestCount()); - } - } - - @Test - public void testRandomAccessFileBodyRetried() - throws IOException, InterruptedException, MinioException { - Path tmpFile = Files.createTempFile("retry-raf-test", ".bin"); - try (MockWebServer server = new MockWebServer()) { - server.enqueue(serverError500Html()); - server.enqueue( - new MockResponse() - .setResponseCode(200) - .setHeader("ETag", "\"abc123\"") - .setHeader("Content-Length", "0")); - server.start(); - - byte[] content = "hello-retry".getBytes(java.nio.charset.StandardCharsets.UTF_8); - Files.write(tmpFile, content); - - try (RandomAccessFile raf = new RandomAccessFile(tmpFile.toFile(), "r")) { - MinioAsyncClient client = - MinioAsyncClient.builder() - .endpoint(server.url("").toString()) - .credentials("access", "secret") - .region("us-east-1") - .maxRetries(2) - .build(); - client - .putObject( - PutObjectAPIArgs.builder() - .bucket("test-bucket") - .object("test-object") - .file(raf, content.length) - .build()) - .join(); - } - - Assert.assertEquals(2, server.getRequestCount()); - byte[] first = server.takeRequest().getBody().readByteArray(); - byte[] second = server.takeRequest().getBody().readByteArray(); - Assert.assertArrayEquals(first, second); - } finally { - Files.deleteIfExists(tmpFile); - } - } - - @Test - public void testRecordedRequestHeaders() - throws IOException, InterruptedException, MinioException { - try (MockWebServer server = new MockWebServer()) { - server.enqueue(serverError500Html()); - server.enqueue(successResponse()); - server.start(); - - MinioClient client = - MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - // Both requests should be to the same path - RecordedRequest first = server.takeRequest(); - RecordedRequest second = server.takeRequest(); - Assert.assertEquals(first.getPath(), second.getPath()); - } - } - - @Test - public void testInvalidResponseExceptionHasResponseCode() { - InvalidResponseException e = new InvalidResponseException(503, "text/html", "body", "trace"); - Assert.assertEquals(503, e.responseCode()); - } } From 6d39a12dbc9cd47a209864492165fc795d9fce2b Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 7 May 2026 19:19:13 +0000 Subject: [PATCH 11/15] feat: restore full retry capability on top of OkHttp interceptor Reinstate the retry triggers, classifiers, and configurability that the strip-back trimmed away, while keeping the orchestration in a single self-contained OkHttp interceptor. - Retry.java: status-code/S3-code sets, IOException filter, and the full-jitter exponential-backoff formula. Defaults: MAX_RETRY=10, DEFAULT_RETRY_UNIT_MS=200, DEFAULT_RETRY_CAP_MS=1000, MAX_JITTER=1.0. - Http.RetryInterceptor: retries on retryable IOException, retryable HTTP status, and retryable S3 in non-2xx response bodies (peek capped at 5 MiB). Per-instance attempt budget via IntSupplier. - BaseS3Client.wrapWithRetry installs the interceptor on every supplied client (default or user-passed) and disables OkHttp's retryOnConnectionFailure to avoid double retry. - Restore the public maxRetries API: MinioClient.setMaxRetries, MinioClient.Builder.maxRetries, MinioAsyncClient.Builder.maxRetries, BaseS3Client.setMaxRetries. Pass 1 to disable retries. - Drop the dead Http.S3Request.body() getter that was orphaned by the earlier strip-back. RetryTest expanded from 2 to 22 tests covering the helpers, the interceptor, every retryable status code, S3-code retry on a non-2xx body, retry exhaustion, mid-sequence retry, post-construction setter, and builder validation. Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - ./gradlew build (api, adminapi, examples, functional) PASS - mint suite, erasure mode, JARs from this commit: PASS in 44.3s --- api/src/main/java/io/minio/BaseS3Client.java | 34 +- api/src/main/java/io/minio/Http.java | 134 +++++-- .../main/java/io/minio/MinioAsyncClient.java | 16 +- api/src/main/java/io/minio/MinioClient.java | 19 + api/src/main/java/io/minio/Retry.java | 128 +++++++ api/src/test/java/io/minio/RetryTest.java | 328 +++++++++++++++++- 6 files changed, 603 insertions(+), 56 deletions(-) create mode 100644 api/src/main/java/io/minio/Retry.java diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 81dff4e04..b039adca2 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -111,19 +111,49 @@ public abstract class BaseS3Client implements AutoCloseable { protected OkHttpClient httpClient; protected boolean closeHttpClient; + /** + * Maximum attempts per S3 request. Default {@link Retry#MAX_RETRY}. Read on every request via the + * retry interceptor's supplier so runtime tuning via {@link #setMaxRetries(int)} takes immediate + * effect. + */ + protected volatile int maxRetries = Retry.MAX_RETRY; + protected BaseS3Client( Http.BaseUrl baseUrl, Provider provider, OkHttpClient httpClient, boolean closeHttpClient) { this.baseUrl = baseUrl; this.provider = provider; - this.httpClient = httpClient; this.closeHttpClient = closeHttpClient; + this.httpClient = wrapWithRetry(httpClient); } protected BaseS3Client(BaseS3Client client) { this.baseUrl = client.baseUrl; this.provider = client.provider; - this.httpClient = client.httpClient; this.closeHttpClient = client.closeHttpClient; + this.maxRetries = client.maxRetries; + this.httpClient = wrapWithRetry(client.httpClient); + } + + /** + * Re-wires the retry interceptor on {@code client} so it reads {@link #maxRetries} from this + * instance. Strips any prior {@link Http.RetryInterceptor} (e.g. one bound to a different + * instance via the copy constructor) before installing this one. + */ + private OkHttpClient wrapWithRetry(OkHttpClient client) { + OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false); + builder.interceptors().removeIf(i -> i instanceof Http.RetryInterceptor); + return builder.addInterceptor(new Http.RetryInterceptor(() -> this.maxRetries)).build(); + } + + /** + * Sets the maximum number of attempts for transient HTTP failures. Pass {@code 1} to disable + * automatic retries. Defaults to {@link Retry#MAX_RETRY}. + * + * @param maxRetries maximum attempts (must be {@code >= 1}). + */ + public void setMaxRetries(int maxRetries) { + if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1"); + this.maxRetries = maxRetries; } /** Closes underneath HTTP client. */ diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index 4c1a65091..9b4709e33 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -52,8 +52,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; +import java.util.function.IntSupplier; import java.util.regex.Matcher; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -622,6 +622,9 @@ public static OkHttpClient newDefaultClient() { .writeTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS) .readTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS) .protocols(Arrays.asList(Protocol.HTTP_1_1)) + // Our RetryInterceptor handles transient failures with full-jitter backoff; defer + // entirely to it instead of layering OkHttp's connection-level retry on top. + .retryOnConnectionFailure(false) .addInterceptor(new RetryInterceptor()) .build(); try { @@ -687,49 +690,115 @@ public static OkHttpClient setTimeout( } /** - * OkHttp interceptor that retries requests on retryable HTTP status codes with exponential - * backoff and full jitter. + * OkHttp interceptor that retries transient HTTP failures with full-jitter exponential backoff. + * + *

Retries on: + * + *

    + *
  • retryable IOException ({@link Retry#isRequestErrorRetryable}) — connection reset, EOF, + * socket timeout, idle-connection close. Excludes TLS handshake / unknown-CA / HTTPS + * protocol mismatch. + *
  • retryable HTTP status code ({@link Retry#RETRYABLE_HTTP_STATUS_CODES}) — 408, 429, 499, + * 500, 502, 503, 504, 520. + *
  • retryable S3 error code in a non-2xx response body ({@link Retry#RETRYABLE_S3_CODES}) — + * {@code SlowDown}, {@code InternalError}, {@code ExpiredToken}, etc. + *
+ * + *

Backoff is computed by {@link Retry#exponentialBackoffMs}. The maximum number of attempts is + * supplied per intercept call via {@link IntSupplier} so that an SDK client can expose runtime + * tuning while keeping the interceptor itself stateless. The no-arg constructor uses the default + * {@link Retry#MAX_RETRY}. + * + *

To opt out of retries on a custom {@link OkHttpClient}, simply do not register this + * interceptor. */ public static class RetryInterceptor implements Interceptor { - private static final int MAX_RETRIES = 5; - private static final long BASE_DELAY_MS = 200L; - private static final long MAX_DELAY_MS = 30_000L; - private static final Set RETRYABLE_STATUS_CODES = - ImmutableSet.of( - 408, // Request Timeout - 429, // Too Many Requests - 499, // Client Closed Request (nginx) - 500, // Internal Server Error - 502, // Bad Gateway - 503, // Service Unavailable - 504, // Gateway Timeout - 520); // Cloudflare unknown error + /** Maximum body bytes inspected when probing for an S3 {@code } value. */ + private static final long MAX_PEEK_BYTES = 5L * 1024L * 1024L; + + private static final java.util.regex.Pattern S3_ERROR_CODE_PATTERN = + java.util.regex.Pattern.compile("([^<]+)"); + + private final IntSupplier maxAttemptsSupplier; + + /** Creates a retry interceptor that uses {@link Retry#MAX_RETRY} attempts. */ + public RetryInterceptor() { + this(() -> Retry.MAX_RETRY); + } + + /** + * Creates a retry interceptor that reads its attempt budget from the supplier on every + * intercept call. Supplier values less than 1 are clamped to 1 (single attempt = retry off). + */ + public RetryInterceptor(IntSupplier maxAttemptsSupplier) { + this.maxAttemptsSupplier = Objects.requireNonNull(maxAttemptsSupplier, "maxAttemptsSupplier"); + } @Override public okhttp3.Response intercept(Chain chain) throws IOException { okhttp3.Request request = chain.request(); - okhttp3.Response response = chain.proceed(request); + int maxAttempts = Math.max(1, maxAttemptsSupplier.getAsInt()); + + okhttp3.Response response = null; + IOException lastException = null; + + for (int attempt = 0; attempt < maxAttempts; attempt++) { + if (attempt > 0) { + long delayMs = Retry.exponentialBackoffMs(attempt - 1); + if (delayMs > 0L) { + try { + Thread.sleep(delayMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new IOException("Retry interrupted", ie); + } + } + } - int tryCount = 0; - while (RETRYABLE_STATUS_CODES.contains(response.code()) && tryCount < MAX_RETRIES) { - tryCount++; - response.close(); + if (response != null) { + response.close(); + response = null; + } - // Cap exponent to avoid bit-shift overflow at high attempt counts. - int exp = Math.min(tryCount, 30); - long backoffCap = Math.min(MAX_DELAY_MS, BASE_DELAY_MS * (1L << exp)); - long jittered = ThreadLocalRandom.current().nextLong(0, Math.max(1, backoffCap)); try { - Thread.sleep(jittered); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - throw new IOException("retry interrupted", ie); + response = chain.proceed(request); + } catch (IOException e) { + if (!Retry.isRequestErrorRetryable(e)) throw e; + lastException = e; + continue; } - response = chain.proceed(request); + if (Retry.isHttpStatusRetryable(response.code())) { + lastException = null; + continue; + } + + if (!response.isSuccessful()) { + String s3Code = peekS3ErrorCode(response); + if (Retry.isS3CodeRetryable(s3Code)) { + lastException = null; + continue; + } + } + + return response; } + + if (lastException != null) throw lastException; return response; } + + /** Returns the S3 {@code } value if the body is XML containing one, else null. */ + private static String peekS3ErrorCode(okhttp3.Response response) { + try { + String body = response.peekBody(MAX_PEEK_BYTES).string(); + if (body.isEmpty() || !body.contains("= 1"); + this.maxRetries = maxRetries; + return this; + } + public MinioAsyncClient build() { Utils.validateNotNull(baseUrl, "endpoint"); @@ -233,7 +244,10 @@ public MinioAsyncClient build() { httpClient = Http.newDefaultClient(); } - return new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient); + MinioAsyncClient client = + new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient); + client.maxRetries = maxRetries; + return client; } } diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java index 03dd587a9..839a14cbd 100644 --- a/api/src/main/java/io/minio/MinioClient.java +++ b/api/src/main/java/io/minio/MinioClient.java @@ -1921,6 +1921,16 @@ public void ignoreCertCheck() throws MinioException { asyncClient.ignoreCertCheck(); } + /** + * Sets the maximum number of attempts for transient HTTP failures. Pass {@code 1} to disable + * automatic retries. Defaults to 10. + * + * @param maxRetries maximum attempts (must be {@code >= 1}). + */ + public void setMaxRetries(int maxRetries) { + asyncClient.setMaxRetries(maxRetries); + } + /** * Sets application's name/version to user agent. For more information about user agent refer #rfc2616. @@ -2040,6 +2050,15 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { return this; } + /** + * Sets the maximum number of attempts per request. Pass {@code 1} to disable automatic retries. + * Defaults to 10. + */ + public Builder maxRetries(int maxRetries) { + asyncClientBuilder.maxRetries(maxRetries); + return this; + } + public MinioClient build() { MinioAsyncClient asyncClient = asyncClientBuilder.build(); return new MinioClient(asyncClient); diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java new file mode 100644 index 000000000..72c06d6c2 --- /dev/null +++ b/api/src/main/java/io/minio/Retry.java @@ -0,0 +1,128 @@ +/* + * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2026 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.minio; + +import com.google.common.collect.ImmutableSet; +import java.io.IOException; +import java.security.cert.CertPathBuilderException; +import java.security.cert.CertPathValidatorException; +import java.security.cert.CertificateException; +import java.util.Set; +import java.util.concurrent.ThreadLocalRandom; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; +import javax.net.ssl.SSLPeerUnverifiedException; + +/** + * Retry configuration and classification helpers used by {@link Http.RetryInterceptor}. + * + *

Defines the retryable HTTP status set, retryable S3 error code set, IOException filter, and + * the full-jitter exponential backoff formula used for transient failure recovery. + */ +class Retry { + /** Default maximum number of attempts per request. */ + static final int MAX_RETRY = 10; + + /** Base unit per retry attempt, in milliseconds. */ + static final long DEFAULT_RETRY_UNIT_MS = 200L; + + /** Per-attempt sleep cap, in milliseconds. */ + static final long DEFAULT_RETRY_CAP_MS = 1_000L; + + /** Maximum jitter fraction in {@code [0.0, 1.0]}. {@code 1.0} = full jitter. */ + static final double MAX_JITTER = 1.0; + + /** Retryable AWS S3 error codes. */ + static final Set RETRYABLE_S3_CODES = + ImmutableSet.of( + "RequestError", + "RequestTimeout", + "Throttling", + "ThrottlingException", + "RequestLimitExceeded", + "RequestThrottled", + "InternalError", + "ExpiredToken", + "ExpiredTokenException", + "SlowDown", + "SlowDownWrite", + "SlowDownRead"); + + /** Retryable HTTP status codes. */ + static final Set RETRYABLE_HTTP_STATUS_CODES = + ImmutableSet.of( + 408, // Request Timeout + 429, // Too Many Requests + 499, // Client Closed Request (nginx) + 500, // Internal Server Error + 502, // Bad Gateway + 503, // Service Unavailable + 504, // Gateway Timeout + 520); // Cloudflare unknown error + + static boolean isS3CodeRetryable(String code) { + return code != null && RETRYABLE_S3_CODES.contains(code); + } + + static boolean isHttpStatusRetryable(int code) { + return RETRYABLE_HTTP_STATUS_CODES.contains(code); + } + + /** + * Returns true if {@code e} represents a transient transport failure that should be retried. TLS + * handshake failure, unknown-CA / cert-path errors, and the "server gave HTTP response to HTTPS + * client" protocol mismatch are NOT retryable; everything else (connection reset, EOF, server + * closed idle connection, socket timeout, …) is. + */ + static boolean isRequestErrorRetryable(IOException e) { + if (e instanceof SSLHandshakeException) return false; + if (e instanceof SSLPeerUnverifiedException) return false; + if (e instanceof SSLException) { + Throwable cause = e.getCause(); + if (cause instanceof CertPathBuilderException + || cause instanceof CertPathValidatorException + || cause instanceof CertificateException) { + return false; + } + } + String msg = e.getMessage(); + if (msg != null && msg.contains("server gave HTTP response to HTTPS client")) return false; + return true; + } + + /** + * Computes the exponential-backoff-with-full-jitter delay for retry {@code attempt} (0-indexed: + * {@code 0} = before the second attempt, {@code 1} = before the third, …): + * + *

+   *   sleep = min(DEFAULT_RETRY_CAP_MS, DEFAULT_RETRY_UNIT_MS * 2^attempt)
+   *   sleep -= random.nextDouble() * sleep * MAX_JITTER     // full jitter when MAX_JITTER == 1.0
+   * 
+ * + *

With {@code MAX_JITTER == 1.0}, returns a uniform random value in {@code [0, min(cap, base * + * 2^attempt)]}. + */ + static long exponentialBackoffMs(int attempt) { + int exp = Math.min(Math.max(attempt, 0), 30); + long sleep = DEFAULT_RETRY_UNIT_MS * (1L << exp); + if (sleep > DEFAULT_RETRY_CAP_MS) sleep = DEFAULT_RETRY_CAP_MS; + sleep -= (long) (ThreadLocalRandom.current().nextDouble() * (double) sleep * MAX_JITTER); + return Math.max(0L, sleep); + } + + private Retry() {} +} diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 9345ef8bd..7c4d7748e 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -17,16 +17,163 @@ package io.minio; import io.minio.errors.ErrorResponseException; +import io.minio.errors.InvalidResponseException; import io.minio.errors.MinioException; import java.io.IOException; +import java.net.SocketException; +import javax.net.ssl.SSLHandshakeException; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okio.Buffer; import org.junit.Assert; import org.junit.Test; -/** Integration tests for {@link Http.RetryInterceptor}. */ +/** Unit + integration tests for {@link Retry} and {@link Http.RetryInterceptor}. */ public class RetryTest { + + // --------------------------------------------------------------------------- + // Retry.isHttpStatusRetryable + // --------------------------------------------------------------------------- + + @Test + public void testIsHttpStatusRetryable_retryable() { + Assert.assertTrue(Retry.isHttpStatusRetryable(408)); + Assert.assertTrue(Retry.isHttpStatusRetryable(429)); + Assert.assertTrue(Retry.isHttpStatusRetryable(499)); + Assert.assertTrue(Retry.isHttpStatusRetryable(500)); + Assert.assertTrue(Retry.isHttpStatusRetryable(502)); + Assert.assertTrue(Retry.isHttpStatusRetryable(503)); + Assert.assertTrue(Retry.isHttpStatusRetryable(504)); + Assert.assertTrue(Retry.isHttpStatusRetryable(520)); + } + + @Test + public void testIsHttpStatusRetryable_notRetryable() { + for (int code : new int[] {200, 201, 204, 301, 304, 400, 401, 403, 404, 409, 412, 416, 501}) { + Assert.assertFalse( + "status " + code + " must not be retryable", Retry.isHttpStatusRetryable(code)); + } + } + + // --------------------------------------------------------------------------- + // Retry.isS3CodeRetryable + // --------------------------------------------------------------------------- + + @Test + public void testIsS3CodeRetryable_retryable() { + String[] codes = { + "RequestError", + "RequestTimeout", + "Throttling", + "ThrottlingException", + "RequestLimitExceeded", + "RequestThrottled", + "InternalError", + "ExpiredToken", + "ExpiredTokenException", + "SlowDown", + "SlowDownWrite", + "SlowDownRead" + }; + for (String c : codes) { + Assert.assertTrue("S3 code " + c + " must be retryable", Retry.isS3CodeRetryable(c)); + } + } + + @Test + public void testIsS3CodeRetryable_notRetryable() { + Assert.assertFalse(Retry.isS3CodeRetryable("NoSuchKey")); + Assert.assertFalse(Retry.isS3CodeRetryable("NoSuchBucket")); + Assert.assertFalse(Retry.isS3CodeRetryable("AccessDenied")); + Assert.assertFalse(Retry.isS3CodeRetryable("RetryHead")); + Assert.assertFalse(Retry.isS3CodeRetryable("")); + Assert.assertFalse(Retry.isS3CodeRetryable(null)); + } + + // --------------------------------------------------------------------------- + // Retry.isRequestErrorRetryable + // --------------------------------------------------------------------------- + + @Test + public void testIsRequestErrorRetryable_retryable() { + Assert.assertTrue(Retry.isRequestErrorRetryable(new IOException("connection reset"))); + Assert.assertTrue(Retry.isRequestErrorRetryable(new IOException("EOF"))); + Assert.assertTrue( + Retry.isRequestErrorRetryable(new IOException("http: server closed idle connection"))); + Assert.assertTrue(Retry.isRequestErrorRetryable(new SocketException("Connection timed out"))); + Assert.assertTrue(Retry.isRequestErrorRetryable(new java.net.SocketTimeoutException("read"))); + } + + @Test + public void testIsRequestErrorRetryable_sslHandshakeNotRetryable() { + Assert.assertFalse( + Retry.isRequestErrorRetryable(new SSLHandshakeException("cert not trusted"))); + } + + @Test + public void testIsRequestErrorRetryable_protocolMismatchNotRetryable() { + Assert.assertFalse( + Retry.isRequestErrorRetryable( + new IOException("server gave HTTP response to HTTPS client"))); + } + + // --------------------------------------------------------------------------- + // Retry.exponentialBackoffMs + // --------------------------------------------------------------------------- + + @Test + public void testExponentialBackoffMs_attempt0WithinFirstUnit() { + // attempt=0: cap = min(1000, 200*2^0) = 200ms; with full jitter, result in [0, 200]. + for (int i = 0; i < 100; i++) { + long delay = Retry.exponentialBackoffMs(0); + Assert.assertTrue( + "attempt 0 delay must be in [0, 200ms], got " + delay, delay >= 0 && delay <= 200); + } + } + + @Test + public void testExponentialBackoffMs_attempt2WithinSecondCap() { + // attempt=2: cap = min(1000, 200*2^2) = 800ms. + for (int i = 0; i < 100; i++) { + long delay = Retry.exponentialBackoffMs(2); + Assert.assertTrue( + "attempt 2 delay must be in [0, 800ms], got " + delay, delay >= 0 && delay <= 800); + } + } + + @Test + public void testExponentialBackoffMs_cappedAtRetryCap() { + // attempt=10: uncapped 200*2^10 = 204800ms; capped at 1000ms. + for (int i = 0; i < 100; i++) { + long delay = Retry.exponentialBackoffMs(10); + Assert.assertTrue("delay must be <= cap, got " + delay, delay <= Retry.DEFAULT_RETRY_CAP_MS); + Assert.assertTrue(delay >= 0); + } + } + + @Test + public void testExponentialBackoffMs_negativeAttemptIsClamped() { + // Negative attempt clamps to 0; cap = 200ms. + long delay = Retry.exponentialBackoffMs(-5); + Assert.assertTrue(delay >= 0 && delay <= 200); + } + + @Test + public void testExponentialBackoffMs_highAttemptDoesNotOverflow() { + // High attempts must not bit-shift overflow; cap saturates. + for (int attempt : new int[] {30, 31, 60, 100, 1000}) { + long delay = Retry.exponentialBackoffMs(attempt); + Assert.assertTrue( + "attempt=" + attempt + " delay must be <= cap, got " + delay, + delay <= Retry.DEFAULT_RETRY_CAP_MS); + Assert.assertTrue("attempt=" + attempt + " delay must be >= 0", delay >= 0); + } + } + + // --------------------------------------------------------------------------- + // Integration tests via MockWebServer + // --------------------------------------------------------------------------- + private static final String LIST_BUCKETS_OK = "" + "" @@ -40,21 +187,34 @@ private MockResponse successResponse() { .setBody(new Buffer().writeUtf8(LIST_BUCKETS_OK)); } - private MockResponse retryableServerError() { + private MockResponse htmlServerError(int code) { return new MockResponse() - .setResponseCode(503) + .setResponseCode(code) .setHeader("Content-Type", "text/html") - .setBody(new Buffer().writeUtf8("Service Unavailable")); + .setBody(new Buffer().writeUtf8("" + code + "")); + } + + private MockResponse xmlError(int code, String s3Code) { + String xml = + "" + + "" + + s3Code + + "m/id"; + return new MockResponse() + .setResponseCode(code) + .setHeader("Content-Type", "application/xml") + .setBody(new Buffer().writeUtf8(xml)); } @Test - public void testRetryOnRetryableStatusThenSuccess() throws IOException, MinioException { + public void testRetryOn503ThenSuccess() throws IOException, MinioException { try (MockWebServer server = new MockWebServer()) { - server.enqueue(retryableServerError()); + server.enqueue(htmlServerError(503)); server.enqueue(successResponse()); server.start(); - MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).build(); + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); client.listBuckets(); Assert.assertEquals(2, server.getRequestCount()); @@ -62,28 +222,160 @@ public void testRetryOnRetryableStatusThenSuccess() throws IOException, MinioExc } @Test - public void testNoRetryOn4xx() throws IOException, MinioException { - String notFoundXml = - "" - + "NoSuchBucketnot found" - + "/abc"; + public void testRetryOnEachRetryableHttpCode() throws IOException, MinioException { + for (int code : new int[] {408, 429, 499, 500, 502, 503, 504, 520}) { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(code)); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals("status " + code + " expected to retry", 2, server.getRequestCount()); + } + } + } + @Test + public void testRetryOnRetryableS3CodeIn400Body() throws IOException, MinioException { + // 400 is NOT a retryable HTTP status, but body has retryable S3 code → retry. try (MockWebServer server = new MockWebServer()) { - server.enqueue( - new MockResponse() - .setResponseCode(404) - .setHeader("Content-Type", "application/xml") - .setBody(new Buffer().writeUtf8(notFoundXml))); + server.enqueue(xmlError(400, "ExpiredToken")); + server.enqueue(successResponse()); server.start(); - MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).build(); + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.listBuckets(); + + Assert.assertEquals(2, server.getRequestCount()); + } + } + + @Test + public void testNoRetryOn404() throws IOException, MinioException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(xmlError(404, "NoSuchBucket")); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { client.listBuckets(); Assert.fail("expected ErrorResponseException"); } catch (ErrorResponseException e) { Assert.assertEquals("NoSuchBucket", e.errorResponse().code()); + Assert.assertEquals(404, e.response().code()); + } + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testNoRetryOn403() throws IOException, MinioException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(xmlError(403, "AccessDenied")); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected ErrorResponseException"); + } catch (ErrorResponseException e) { + Assert.assertEquals("AccessDenied", e.errorResponse().code()); + } + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test + public void testRetryExhaustedReturnsLastResponse() throws IOException, MinioException { + // 3 attempts, all 500 (HTML so server-side response dispatches to InvalidResponseException). + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(500)); + server.enqueue(htmlServerError(500)); + server.enqueue(htmlServerError(500)); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected exception after exhausted retries"); + } catch (InvalidResponseException e) { + // expected — non-XML 500 + } + Assert.assertEquals(3, server.getRequestCount()); + } + } + + @Test + public void testMaxRetriesOneDisablesRetry() throws IOException, MinioException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(503)); + // Second response should never be reached. + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(1).build(); + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (InvalidResponseException e) { + // expected } Assert.assertEquals(1, server.getRequestCount()); } } + + @Test + public void testSetMaxRetriesPostConstruction() throws IOException, MinioException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(503)); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); + client.setMaxRetries(1); // disable + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (InvalidResponseException e) { + // expected + } + Assert.assertEquals(1, server.getRequestCount()); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testMaxRetriesBuilderValidation() { + MinioClient.builder().endpoint("http://localhost:9000").maxRetries(0).build(); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetMaxRetriesValidation() { + MinioClient client = MinioClient.builder().endpoint("http://localhost:9000").build(); + client.setMaxRetries(0); + } + + @Test + public void testMultipleRetrySucceedsOnThirdAttempt() throws IOException, MinioException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(500)); + server.enqueue(htmlServerError(503)); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + client.listBuckets(); + + Assert.assertEquals(3, server.getRequestCount()); + } + } } From 756e2942e6e21405d249502aba4bb66ee3545bb5 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 7 May 2026 19:24:35 +0000 Subject: [PATCH 12/15] docs(retry): clarify RetryInterceptor as supported API; add terminal-status assertions Address Copilot review feedback (PR #1701, review 4246873092): - RetryInterceptor Javadoc now declares it as part of the SDK's supported public API and documents how BaseS3Client wraps every supplied OkHttpClient. Drops {@link} references to package-private Retry helpers so the public Javadoc no longer points at internal symbols. Adds a Threading note about Thread.sleep blocking the OkHttp dispatcher slot during backoff and points high-concurrency callers at Dispatcher pool sizing. - BaseS3Client.executeAsync Javadoc reworded to reflect the wrapWithRetry behaviour (interceptor installed on every supplied client, default or caller-provided) and to note the maxRetries supplier is read per request. - RetryTest: strengthen testRetryExhaustedReturnsLastResponse to assert the InvalidResponseException reflects HTTP 500 rather than allowing any exception type to pass. Add testRetryExhaustedSurfacesXmlErrorResponse for the XML/ErrorResponseException path, asserting both response.code() and the parsed S3 code after retries are exhausted. Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - :api:javadoc PASS --- api/src/main/java/io/minio/BaseS3Client.java | 7 +++-- api/src/main/java/io/minio/Http.java | 32 ++++++++++++-------- api/src/test/java/io/minio/RetryTest.java | 30 +++++++++++++++++- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index b039adca2..589d2f618 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -297,8 +297,11 @@ private String[] handleRedirectResponse( } /** - * Execute HTTP request asynchronously for given parameters. Retries on retryable HTTP status - * codes are handled by {@link Http.RetryInterceptor} installed on the default HTTP client. + * Execute HTTP request asynchronously for given parameters. Retries on retryable IOException, + * HTTP status, and S3 error code are handled by {@link Http.RetryInterceptor}, which {@link + * #wrapWithRetry} installs on the underlying {@link OkHttpClient} regardless of whether the + * client is the default or caller-supplied. The attempt budget is read from {@link #maxRetries} + * on every request. */ protected CompletableFuture executeAsync(Http.S3Request s3request, String region) { Credentials credentials = (provider == null) ? null : provider.fetch(); diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index 9b4709e33..0d050ac8d 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -692,24 +692,32 @@ public static OkHttpClient setTimeout( /** * OkHttp interceptor that retries transient HTTP failures with full-jitter exponential backoff. * + *

This is part of the SDK's supported public API: it is installed automatically by {@link + * BaseS3Client} on every supplied {@link OkHttpClient} (default or caller-provided), and may also + * be registered explicitly on a stand-alone {@link OkHttpClient} via {@code .addInterceptor(new + * Http.RetryInterceptor())}. + * *

Retries on: * *

    - *
  • retryable IOException ({@link Retry#isRequestErrorRetryable}) — connection reset, EOF, - * socket timeout, idle-connection close. Excludes TLS handshake / unknown-CA / HTTPS - * protocol mismatch. - *
  • retryable HTTP status code ({@link Retry#RETRYABLE_HTTP_STATUS_CODES}) — 408, 429, 499, - * 500, 502, 503, 504, 520. - *
  • retryable S3 error code in a non-2xx response body ({@link Retry#RETRYABLE_S3_CODES}) — - * {@code SlowDown}, {@code InternalError}, {@code ExpiredToken}, etc. + *
  • retryable IOException — connection reset, EOF, socket timeout, idle-connection close. + * Excludes TLS handshake / unknown-CA / HTTPS protocol mismatch. + *
  • retryable HTTP status code — 408, 429, 499, 500, 502, 503, 504, 520. + *
  • retryable S3 error code in a non-2xx response body — {@code SlowDown}, {@code + * InternalError}, {@code ExpiredToken}, etc. *
* - *

Backoff is computed by {@link Retry#exponentialBackoffMs}. The maximum number of attempts is - * supplied per intercept call via {@link IntSupplier} so that an SDK client can expose runtime - * tuning while keeping the interceptor itself stateless. The no-arg constructor uses the default - * {@link Retry#MAX_RETRY}. + *

Backoff is full-jitter exponential, with a 200 ms unit and a 1 s per-attempt cap. + * The maximum number of attempts is supplied per intercept call via {@link IntSupplier} so that + * an SDK client can expose runtime tuning while keeping the interceptor itself stateless. The + * no-arg constructor uses the package default of 10 attempts. + * + *

Threading. Backoff sleeps on the OkHttp dispatcher thread that owns the call. Under + * sustained 5xx/429 storms this can hold dispatcher slots idle while waiting. Callers that need + * higher concurrency under widespread retry should size the dispatcher worker pool accordingly + * (see {@link okhttp3.Dispatcher#setMaxRequests} / {@code setMaxRequestsPerHost}). * - *

To opt out of retries on a custom {@link OkHttpClient}, simply do not register this + *

To opt out of retries on a stand-alone {@link OkHttpClient}, simply do not register this * interceptor. */ public static class RetryInterceptor implements Interceptor { diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 7c4d7748e..0f3fcf1e0 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -306,7 +306,35 @@ public void testRetryExhaustedReturnsLastResponse() throws IOException, MinioExc client.listBuckets(); Assert.fail("expected exception after exhausted retries"); } catch (InvalidResponseException e) { - // expected — non-XML 500 + // Terminal exception must surface the underlying 500 status so callers can distinguish + // exhausted retries from unrelated failures (e.g. NPE, parser bugs). + Assert.assertTrue( + "exhausted-retries exception must reflect HTTP 500, got: " + e.getMessage(), + e.getMessage().contains("Response code: 500")); + } + Assert.assertEquals(3, server.getRequestCount()); + } + } + + @Test + public void testRetryExhaustedSurfacesXmlErrorResponse() throws IOException, MinioException { + // 3 attempts, all 503 with XML InternalError — confirms the terminal + // ErrorResponseException carries both the 503 status and the parsed S3 code after the retry + // loop gives up. + try (MockWebServer server = new MockWebServer()) { + server.enqueue(xmlError(503, "InternalError")); + server.enqueue(xmlError(503, "InternalError")); + server.enqueue(xmlError(503, "InternalError")); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.fail("expected ErrorResponseException after exhausted retries"); + } catch (ErrorResponseException e) { + Assert.assertEquals(503, e.response().code()); + Assert.assertEquals("InternalError", e.errorResponse().code()); } Assert.assertEquals(3, server.getRequestCount()); } From 2f2fb4f9507d4032500e0537e32bd5e22487c0be Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 7 May 2026 20:04:45 +0000 Subject: [PATCH 13/15] fix(retry): cancellation awareness, broader docs, tighter scope, more tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address findings from the deep-review pass on the retry feature. Behaviour - RetryInterceptor.intercept now consults chain.call().isCanceled() at the top of each attempt and after every IOException from chain.proceed. Without this a cancelled call would surface IOException("Canceled"), fall through Retry.isRequestErrorRetryable as "retryable", and burn the full attempt budget in sleep+retry — a regression in real OkHttp cancellation flows. Mirrors minio-go's `errors.Is(err, context.Canceled)` short-circuit. API hardening - BaseS3Client.maxRetries narrowed from protected to package-private; the only out-of-class writer is MinioAsyncClient.Builder (same package), which now goes through the validating setMaxRetries() instead of a raw field assignment. - Http.Body(okhttp3.RequestBody) now validates non-null and documents the retry-replay caveat for one-shot bodies. Documentation - RetryInterceptor Javadoc adds explicit notes on cancellation handling, request-body replayability (with a warning about caller-supplied isOneShot bodies), and the deliberate divergence from minio-go around per-attempt request rebuild / re-signing / region & STS refresh. - BaseS3Client.wrapWithRetry Javadoc states that retryOnConnectionFailure is forced to false on every supplied client. - BaseS3Client.traceOn Javadoc notes per-retry attempts are not surfaced to the trace stream and points at OkHttp's HttpLoggingInterceptor. - MinioAsyncClient.Builder.httpClient overloads now document the wrap behaviour. - Retry.java header references minio-go's retry.go as the contract source. - docs/API.md builder-methods table gains a maxRetries() row. Tests (RetryTest.java) - All 13 integration tests now close the MinioClient via a closeQuietly helper, eliminating the per-test dispatcher leak. - New: testCancelDuringRetryStopsLoop — wraps a real OkHttpClient with the interceptor, fires Call.cancel() ~150ms in, asserts the loop short- circuits in <5s (without the fix it would run all 10 attempts). - New: testRetryOnConnectionDropThenSuccess — exercises the IOException retry path end-to-end via MockWebServer SocketPolicy.DISCONNECT_AT_START. - New: testUserSuppliedClientStillRetries — caller-supplied OkHttpClient with retryOnConnectionFailure(true) still gets the SDK interceptor. - New: testIsRequestErrorRetryable_certPathErrorsNotRetryable — covers the SSLException-with-cert-path-cause and SSLPeerUnverifiedException branches that were previously untested. - New: testMaxRetriesBuilderValidation_negative, testSetMaxRetriesValidation_negative — extends validation coverage beyond the 0-only case. Style - Replaced fully-qualified java.util.regex.Pattern/Matcher with imports on Http.java. - Bumped 2025-only copyright headers on touched files to 2025-2026 (and 2022 -> 2022-2026, 2015-2021 -> 2015-2026). Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - :api:javadoc PASS - ./gradlew build (api, adminapi, examples, functional) PASS minio-go contract: cancellation handling is *added*, bringing parity with minio-go's errors.Is(context.Canceled) check. No retryable status, S3 code, backoff formula, or jitter constant was changed. --- api/src/main/java/io/minio/BaseS3Client.java | 18 +- api/src/main/java/io/minio/Http.java | 52 +++- .../main/java/io/minio/MinioAsyncClient.java | 14 +- api/src/main/java/io/minio/MinioClient.java | 2 +- api/src/main/java/io/minio/Retry.java | 4 + api/src/test/java/io/minio/RetryTest.java | 284 +++++++++++++++--- docs/API.md | 1 + 7 files changed, 316 insertions(+), 59 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index 589d2f618..b53d947c5 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -1,6 +1,6 @@ /* * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, - * (C) 2025 MinIO, Inc. + * (C) 2025-2026 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -114,9 +114,10 @@ public abstract class BaseS3Client implements AutoCloseable { /** * Maximum attempts per S3 request. Default {@link Retry#MAX_RETRY}. Read on every request via the * retry interceptor's supplier so runtime tuning via {@link #setMaxRetries(int)} takes immediate - * effect. + * effect. Package-private so the {@code Builder} classes in this package can seed it via the + * setter; subclasses must go through {@link #setMaxRetries(int)}. */ - protected volatile int maxRetries = Retry.MAX_RETRY; + volatile int maxRetries = Retry.MAX_RETRY; protected BaseS3Client( Http.BaseUrl baseUrl, Provider provider, OkHttpClient httpClient, boolean closeHttpClient) { @@ -138,6 +139,12 @@ protected BaseS3Client(BaseS3Client client) { * Re-wires the retry interceptor on {@code client} so it reads {@link #maxRetries} from this * instance. Strips any prior {@link Http.RetryInterceptor} (e.g. one bound to a different * instance via the copy constructor) before installing this one. + * + *

Also forces {@code retryOnConnectionFailure(false)} on the underlying client. The SDK's own + * {@link Http.RetryInterceptor} is the single source of retry policy; layering OkHttp's + * connection-level retry on top would double-count attempts and obscure the {@link #maxRetries} + * budget. This silently overrides any {@code retryOnConnectionFailure(true)} that a + * caller-supplied client was configured with — by design. */ private OkHttpClient wrapWithRetry(OkHttpClient client) { OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false); @@ -210,6 +217,11 @@ public void setAppInfo(String name, String version) { /** * Enables HTTP call tracing and written to traceStream. * + *

Retry caveat. Tracing happens at the SDK callback level, so only the final response + * of a retry sequence is recorded. Per-attempt traces (which the {@link Http.RetryInterceptor} + * sees and discards) are not surfaced here. To inspect individual retry attempts, register an + * OkHttp {@code HttpLoggingInterceptor} on a custom client. + * * @param traceStream {@link OutputStream} for writing HTTP call tracing. * @see #traceOff */ diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index 0d050ac8d..b33d9c013 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -1,5 +1,5 @@ /* - * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2025 MinIO, Inc. + * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, (C) 2025-2026 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -55,6 +55,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.IntSupplier; import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nonnull; @@ -717,6 +718,28 @@ public static OkHttpClient setTimeout( * higher concurrency under widespread retry should size the dispatcher worker pool accordingly * (see {@link okhttp3.Dispatcher#setMaxRequests} / {@code setMaxRequestsPerHost}). * + *

Cancellation. The interceptor checks {@code chain.call().isCanceled()} before each + * attempt and after any thrown {@link IOException}, so a {@code Call.cancel()} from the caller + * (or an upstream interceptor) terminates the retry loop instead of treating the cancellation as + * a retryable transport error. + * + *

Replayability. Retries call {@code chain.proceed(request)} again, which re-invokes + * {@code request.body().writeTo(sink)}. Callers using the SDK's own body types ({@link Body} for + * {@code byte[]}, {@link ByteBuffer}, or {@link RandomAccessFile}) are safe — those bodies are + * replayable. A caller-supplied {@link okhttp3.RequestBody} that overrides {@code isOneShot()} to + * return {@code true} (or otherwise consumes its source on the first {@code writeTo}) MUST NOT be + * retried; either disable retries via {@link BaseS3Client#setMaxRetries(int)} {@code (1)} for + * those calls, or wrap the body in a replayable form before submission. + * + *

Request reuse. The interceptor passes the same signed {@link okhttp3.Request} on + * every attempt, so the {@code X-Amz-Date} / {@code Authorization} headers are not refreshed + * between attempts. This is harmless for the default attempt budget (worst-case wall-clock + * < 10 s, well inside the 15-minute S3 signing window) but extreme {@code + * maxRetries} values combined with high backoff caps could outlast either the signing window or a + * short-lived STS credential. minio-go's request-level retry rebuilds and re-signs each attempt; + * that semantic is intentionally not replicated here in exchange for the simpler interceptor + * model. + * *

To opt out of retries on a stand-alone {@link OkHttpClient}, simply do not register this * interceptor. */ @@ -724,8 +747,7 @@ public static class RetryInterceptor implements Interceptor { /** Maximum body bytes inspected when probing for an S3 {@code } value. */ private static final long MAX_PEEK_BYTES = 5L * 1024L * 1024L; - private static final java.util.regex.Pattern S3_ERROR_CODE_PATTERN = - java.util.regex.Pattern.compile("([^<]+)"); + private static final Pattern S3_ERROR_CODE_PATTERN = Pattern.compile("([^<]+)"); private final IntSupplier maxAttemptsSupplier; @@ -751,6 +773,14 @@ public okhttp3.Response intercept(Chain chain) throws IOException { IOException lastException = null; for (int attempt = 0; attempt < maxAttempts; attempt++) { + // Honour caller cancellation: mirrors minio-go's + // `errors.Is(err, context.Canceled)` short-circuit so the loop does not + // burn attempts re-issuing a call the caller has already abandoned. + if (chain.call().isCanceled()) { + if (response != null) response.close(); + throw new IOException("Canceled"); + } + if (attempt > 0) { long delayMs = Retry.exponentialBackoffMs(attempt - 1); if (delayMs > 0L) { @@ -771,6 +801,9 @@ public okhttp3.Response intercept(Chain chain) throws IOException { try { response = chain.proceed(request); } catch (IOException e) { + // A cancelled call surfaces as IOException with a message that would + // otherwise pass `isRequestErrorRetryable`. Bail out instead. + if (chain.call().isCanceled()) throw e; if (!Retry.isRequestErrorRetryable(e)) throw e; lastException = e; continue; @@ -801,7 +834,7 @@ private static String peekS3ErrorCode(okhttp3.Response response) { try { String body = response.peekBody(MAX_PEEK_BYTES).string(); if (body.isEmpty() || !body.contains("Retry caveat. {@link Http.RetryInterceptor} re-invokes {@code writeTo} on each + * retry. Pass only replayable bodies; do not pass a body that overrides {@code isOneShot()} to + * return {@code true} (or otherwise consumes its source on first write) unless retries are + * disabled for that call. + */ public Body(okhttp3.RequestBody requestBody) { - this.requestBody = requestBody; + this.requestBody = Utils.validateNotNull(requestBody, "request body"); this.contentType = requestBody.contentType(); } diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java index 9c923f478..8cad70694 100644 --- a/api/src/main/java/io/minio/MinioAsyncClient.java +++ b/api/src/main/java/io/minio/MinioAsyncClient.java @@ -1,6 +1,6 @@ /* * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, - * (C) 2022 MinIO, Inc. + * (C) 2022-2026 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -206,12 +206,22 @@ public Builder credentialsProvider(Provider provider) { return this; } + /** + * Supplies a custom {@link OkHttpClient}. The SDK wraps this client to install {@link + * Http.RetryInterceptor} and forces {@code retryOnConnectionFailure(false)} so that {@link + * Http.RetryInterceptor} owns the entire retry policy. Any prior {@code RetryInterceptor} on + * the supplied client is replaced. + */ public Builder httpClient(OkHttpClient httpClient) { Utils.validateNotNull(httpClient, "http client"); this.httpClient = httpClient; return this; } + /** + * Same as {@link #httpClient(OkHttpClient)} but additionally controls whether the SDK shuts + * down the underlying dispatcher on {@link MinioAsyncClient#close()}. + */ public Builder httpClient(OkHttpClient httpClient, boolean close) { Utils.validateNotNull(httpClient, "http client"); this.httpClient = httpClient; @@ -246,7 +256,7 @@ public MinioAsyncClient build() { MinioAsyncClient client = new MinioAsyncClient(baseUrl, provider, httpClient, closeHttpClient); - client.maxRetries = maxRetries; + client.setMaxRetries(maxRetries); return client; } } diff --git a/api/src/main/java/io/minio/MinioClient.java b/api/src/main/java/io/minio/MinioClient.java index 839a14cbd..f37988225 100644 --- a/api/src/main/java/io/minio/MinioClient.java +++ b/api/src/main/java/io/minio/MinioClient.java @@ -1,6 +1,6 @@ /* * MinIO Java SDK for Amazon S3 Compatible Cloud Storage, - * (C) 2015-2021 MinIO, Inc. + * (C) 2015-2026 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java index 72c06d6c2..9d6a8b56b 100644 --- a/api/src/main/java/io/minio/Retry.java +++ b/api/src/main/java/io/minio/Retry.java @@ -32,6 +32,10 @@ * *

Defines the retryable HTTP status set, retryable S3 error code set, IOException filter, and * the full-jitter exponential backoff formula used for transient failure recovery. + * + *

Constants and predicates here mirror the contract in minio-go's {@code retry.go} + * (https://github.com/minio/minio-go/blob/master/retry.go) so a Java caller experiences the same + * retry semantics as a Go caller. Future drift in minio-go should be reflected here. */ class Retry { /** Default maximum number of attempts per request. */ diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index 0f3fcf1e0..e5b508b57 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -21,9 +21,17 @@ import io.minio.errors.MinioException; import java.io.IOException; import java.net.SocketException; +import java.security.cert.CertPathBuilderException; +import java.security.cert.CertPathValidatorException; +import java.security.cert.CertificateException; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; +import javax.net.ssl.SSLPeerUnverifiedException; +import okhttp3.Call; +import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; import okio.Buffer; import org.junit.Assert; import org.junit.Test; @@ -31,6 +39,15 @@ /** Unit + integration tests for {@link Retry} and {@link Http.RetryInterceptor}. */ public class RetryTest { + private static void closeQuietly(MinioClient client) { + if (client == null) return; + try { + client.close(); + } catch (Exception ignored) { + // Tests do not rely on close(); swallowing keeps the throws clauses narrow. + } + } + // --------------------------------------------------------------------------- // Retry.isHttpStatusRetryable // --------------------------------------------------------------------------- @@ -215,9 +232,12 @@ public void testRetryOn503ThenSuccess() throws IOException, MinioException { MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); + try { + client.listBuckets(); + Assert.assertEquals(2, server.getRequestCount()); + } finally { + closeQuietly(client); + } } } @@ -231,9 +251,12 @@ public void testRetryOnEachRetryableHttpCode() throws IOException, MinioExceptio MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals("status " + code + " expected to retry", 2, server.getRequestCount()); + try { + client.listBuckets(); + Assert.assertEquals("status " + code + " expected to retry", 2, server.getRequestCount()); + } finally { + closeQuietly(client); + } } } } @@ -248,9 +271,12 @@ public void testRetryOnRetryableS3CodeIn400Body() throws IOException, MinioExcep MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); - client.listBuckets(); - - Assert.assertEquals(2, server.getRequestCount()); + try { + client.listBuckets(); + Assert.assertEquals(2, server.getRequestCount()); + } finally { + closeQuietly(client); + } } } @@ -263,13 +289,17 @@ public void testNoRetryOn404() throws IOException, MinioException { MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { - client.listBuckets(); - Assert.fail("expected ErrorResponseException"); - } catch (ErrorResponseException e) { - Assert.assertEquals("NoSuchBucket", e.errorResponse().code()); - Assert.assertEquals(404, e.response().code()); + try { + client.listBuckets(); + Assert.fail("expected ErrorResponseException"); + } catch (ErrorResponseException e) { + Assert.assertEquals("NoSuchBucket", e.errorResponse().code()); + Assert.assertEquals(404, e.response().code()); + } + Assert.assertEquals(1, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(1, server.getRequestCount()); } } @@ -282,12 +312,16 @@ public void testNoRetryOn403() throws IOException, MinioException { MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { - client.listBuckets(); - Assert.fail("expected ErrorResponseException"); - } catch (ErrorResponseException e) { - Assert.assertEquals("AccessDenied", e.errorResponse().code()); + try { + client.listBuckets(); + Assert.fail("expected ErrorResponseException"); + } catch (ErrorResponseException e) { + Assert.assertEquals("AccessDenied", e.errorResponse().code()); + } + Assert.assertEquals(1, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(1, server.getRequestCount()); } } @@ -303,16 +337,20 @@ public void testRetryExhaustedReturnsLastResponse() throws IOException, MinioExc MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { - client.listBuckets(); - Assert.fail("expected exception after exhausted retries"); - } catch (InvalidResponseException e) { - // Terminal exception must surface the underlying 500 status so callers can distinguish - // exhausted retries from unrelated failures (e.g. NPE, parser bugs). - Assert.assertTrue( - "exhausted-retries exception must reflect HTTP 500, got: " + e.getMessage(), - e.getMessage().contains("Response code: 500")); + try { + client.listBuckets(); + Assert.fail("expected exception after exhausted retries"); + } catch (InvalidResponseException e) { + // Terminal exception must surface the underlying 500 status so callers can distinguish + // exhausted retries from unrelated failures (e.g. NPE, parser bugs). + Assert.assertTrue( + "exhausted-retries exception must reflect HTTP 500, got: " + e.getMessage(), + e.getMessage().contains("Response code: 500")); + } + Assert.assertEquals(3, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(3, server.getRequestCount()); } } @@ -330,13 +368,17 @@ public void testRetryExhaustedSurfacesXmlErrorResponse() throws IOException, Min MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { - client.listBuckets(); - Assert.fail("expected ErrorResponseException after exhausted retries"); - } catch (ErrorResponseException e) { - Assert.assertEquals(503, e.response().code()); - Assert.assertEquals("InternalError", e.errorResponse().code()); + try { + client.listBuckets(); + Assert.fail("expected ErrorResponseException after exhausted retries"); + } catch (ErrorResponseException e) { + Assert.assertEquals(503, e.response().code()); + Assert.assertEquals("InternalError", e.errorResponse().code()); + } + Assert.assertEquals(3, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(3, server.getRequestCount()); } } @@ -351,12 +393,16 @@ public void testMaxRetriesOneDisablesRetry() throws IOException, MinioException MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(1).build(); try { - client.listBuckets(); - Assert.fail("expected exception"); - } catch (InvalidResponseException e) { - // expected + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (InvalidResponseException e) { + // expected + } + Assert.assertEquals(1, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(1, server.getRequestCount()); } } @@ -371,12 +417,16 @@ public void testSetMaxRetriesPostConstruction() throws IOException, MinioExcepti MinioClient.builder().endpoint(server.url("").toString()).maxRetries(2).build(); client.setMaxRetries(1); // disable try { - client.listBuckets(); - Assert.fail("expected exception"); - } catch (InvalidResponseException e) { - // expected + try { + client.listBuckets(); + Assert.fail("expected exception"); + } catch (InvalidResponseException e) { + // expected + } + Assert.assertEquals(1, server.getRequestCount()); + } finally { + closeQuietly(client); } - Assert.assertEquals(1, server.getRequestCount()); } } @@ -401,9 +451,149 @@ public void testMultipleRetrySucceedsOnThirdAttempt() throws IOException, MinioE MinioClient client = MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); - client.listBuckets(); + try { + client.listBuckets(); + Assert.assertEquals(3, server.getRequestCount()); + } finally { + closeQuietly(client); + } + } + } - Assert.assertEquals(3, server.getRequestCount()); + // --------------------------------------------------------------------------- + // Additional coverage: SSL cert-path classifier, negative-validation, IOException + // path, user-supplied OkHttpClient, and cancellation short-circuit. + // --------------------------------------------------------------------------- + + @Test + public void testIsRequestErrorRetryable_certPathErrorsNotRetryable() { + // SSLException whose cause is one of the cert-path error types must NOT retry. + SSLException certPathBuilder = new SSLException("x", new CertPathBuilderException("bad")); + SSLException certPathValidator = new SSLException("x", new CertPathValidatorException("bad")); + SSLException certError = new SSLException("x", new CertificateException("bad")); + Assert.assertFalse(Retry.isRequestErrorRetryable(certPathBuilder)); + Assert.assertFalse(Retry.isRequestErrorRetryable(certPathValidator)); + Assert.assertFalse(Retry.isRequestErrorRetryable(certError)); + + // SSLPeerUnverifiedException must NOT retry. + Assert.assertFalse(Retry.isRequestErrorRetryable(new SSLPeerUnverifiedException("untrusted"))); + + // SSLException with an unrelated cause is still retryable (transient TLS hiccup). + Assert.assertTrue( + Retry.isRequestErrorRetryable(new SSLException("read", new IOException("boom")))); + } + + @Test(expected = IllegalArgumentException.class) + public void testMaxRetriesBuilderValidation_negative() { + MinioClient.builder().endpoint("http://localhost:9000").maxRetries(-1).build(); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetMaxRetriesValidation_negative() { + MinioClient client = MinioClient.builder().endpoint("http://localhost:9000").build(); + try { + client.setMaxRetries(-1); + } finally { + closeQuietly(client); + } + } + + @Test + public void testRetryOnConnectionDropThenSuccess() throws IOException, MinioException { + // Simulate a transient transport failure: server drops the socket on first attempt, returns + // the success body on the second. Verifies the IOException retry path end-to-end (only the + // predicate was previously tested in isolation). + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START)); + server.enqueue(successResponse()); + server.start(); + + MinioClient client = + MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); + try { + client.listBuckets(); + Assert.assertEquals(2, server.getRequestCount()); + } finally { + closeQuietly(client); + } + } + } + + @Test + public void testUserSuppliedClientStillRetries() throws IOException, MinioException { + // Caller supplies a custom OkHttpClient with no RetryInterceptor and + // retryOnConnectionFailure(true). wrapWithRetry must still install the SDK interceptor and + // honour maxRetries — proves the integration path for caller-supplied clients. + try (MockWebServer server = new MockWebServer()) { + server.enqueue(htmlServerError(503)); + server.enqueue(htmlServerError(503)); + server.enqueue(successResponse()); + server.start(); + + OkHttpClient custom = new OkHttpClient.Builder().retryOnConnectionFailure(true).build(); + MinioClient client = + MinioClient.builder() + .endpoint(server.url("").toString()) + .httpClient(custom, false) + .maxRetries(3) + .build(); + try { + client.listBuckets(); + Assert.assertEquals(3, server.getRequestCount()); + } finally { + closeQuietly(client); + } + } + } + + @Test(timeout = 10_000) + public void testCancelDuringRetryStopsLoop() throws IOException, InterruptedException { + // Without cancellation handling the interceptor would loop maxRetries=10 times with backoff + // (~6s of sleep). With the chain.call().isCanceled() check, cancel mid-loop must short-circuit + // and surface IOException quickly — the timeout guards against regression. + try (MockWebServer server = new MockWebServer()) { + for (int i = 0; i < 20; i++) { + server.enqueue(htmlServerError(503)); + } + server.start(); + + OkHttpClient client = + new OkHttpClient.Builder() + .retryOnConnectionFailure(false) + .addInterceptor(new Http.RetryInterceptor()) + .build(); + try { + okhttp3.Request request = new okhttp3.Request.Builder().url(server.url("/x")).get().build(); + Call call = client.newCall(request); + + Thread canceler = + new Thread( + () -> { + try { + Thread.sleep(150); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + call.cancel(); + }); + canceler.start(); + + long start = System.currentTimeMillis(); + try { + call.execute().close(); + Assert.fail("expected IOException after cancel"); + } catch (IOException e) { + // expected: cancel surfaces as IOException + } + long elapsed = System.currentTimeMillis() - start; + canceler.join(); + + Assert.assertTrue( + "cancel must short-circuit the retry loop, took " + elapsed + "ms", elapsed < 5_000); + } finally { + client.dispatcher().executorService().shutdown(); + client.connectionPool().evictAll(); + } } } } diff --git a/docs/API.md b/docs/API.md index ee72ac8f1..25101d911 100644 --- a/docs/API.md +++ b/docs/API.md @@ -71,6 +71,7 @@ MinIO Client Builder is used to create MinIO client. Builder has below methods t | `credentials()` | Accepts access key (aka user ID) and secret key (aka password) of an account in S3 service. | | `region()` | Accepts region name of S3 service. If specified, all operations use this region otherwise region is probed per bucket. | | `httpClient()` | Custom HTTP client to override default. | +| `maxRetries()` | Maximum number of attempts per request for transient HTTP failures (retryable status codes 408/429/499/500/502/503/504/520, retryable S3 codes such as `SlowDown`/`InternalError`/`ExpiredToken`, and retryable IOExceptions). Pass `1` to disable automatic retries. Defaults to `10`. | __Examples__ From 7fe6e8744c0d8b36dff953f51ffdfea14f81013a Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 7 May 2026 22:11:15 +0000 Subject: [PATCH 14/15] fix(retry): drop public Javadoc links to package-private Retry; strip prior interceptor from network chain too; correct backoff range docs Address Copilot review (PR #1701, review 4247997916): - BaseS3Client.setMaxRetries and MinioAsyncClient.Builder.maxRetries Javadocs no longer reference {@link Retry#MAX_RETRY}. The Retry class is package-private, so the link target was unresolvable from generated public docs. Replaced with the literal default {@code 10}, which is what callers actually see. - BaseS3Client.wrapWithRetry now also strips any prior RetryInterceptor from networkInterceptors(), not only the application-interceptor list. Previously the Javadoc claimed every prior RetryInterceptor was replaced, but only application interceptors were. A caller that had (mis)registered RetryInterceptor as a network interceptor would have ended up with two retry layers. Defends against that and brings the Javadoc claim into line with the code. - Retry.exponentialBackoffMs Javadoc previously stated the result range as [0, sleep]. The actual range is [1, sleep] because nextDouble() is in [0.0, 1.0) and the (long) cast truncates rand * sleep to at most sleep - 1, so the subtraction never reaches sleep itself. minio-go has the same off-by-one (Float64() is also [0.0, 1.0)); this is documented now rather than papered over, so callers reading the formula see the same bounds the implementation produces. Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - :api:javadoc PASS --- api/src/main/java/io/minio/BaseS3Client.java | 9 ++++++--- api/src/main/java/io/minio/MinioAsyncClient.java | 5 +++-- api/src/main/java/io/minio/Retry.java | 10 +++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index b53d947c5..f4cc5d115 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -137,8 +137,10 @@ protected BaseS3Client(BaseS3Client client) { /** * Re-wires the retry interceptor on {@code client} so it reads {@link #maxRetries} from this - * instance. Strips any prior {@link Http.RetryInterceptor} (e.g. one bound to a different - * instance via the copy constructor) before installing this one. + * instance. Strips any prior {@link Http.RetryInterceptor} from BOTH the application-interceptor + * and network-interceptor chains (e.g. one bound to a different instance via the copy + * constructor, or accidentally registered on the network chain) before installing this one as an + * application interceptor. * *

Also forces {@code retryOnConnectionFailure(false)} on the underlying client. The SDK's own * {@link Http.RetryInterceptor} is the single source of retry policy; layering OkHttp's @@ -149,12 +151,13 @@ protected BaseS3Client(BaseS3Client client) { private OkHttpClient wrapWithRetry(OkHttpClient client) { OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false); builder.interceptors().removeIf(i -> i instanceof Http.RetryInterceptor); + builder.networkInterceptors().removeIf(i -> i instanceof Http.RetryInterceptor); return builder.addInterceptor(new Http.RetryInterceptor(() -> this.maxRetries)).build(); } /** * Sets the maximum number of attempts for transient HTTP failures. Pass {@code 1} to disable - * automatic retries. Defaults to {@link Retry#MAX_RETRY}. + * automatic retries. Defaults to {@code 10}. * * @param maxRetries maximum attempts (must be {@code >= 1}). */ diff --git a/api/src/main/java/io/minio/MinioAsyncClient.java b/api/src/main/java/io/minio/MinioAsyncClient.java index 8cad70694..daebb9052 100644 --- a/api/src/main/java/io/minio/MinioAsyncClient.java +++ b/api/src/main/java/io/minio/MinioAsyncClient.java @@ -210,7 +210,8 @@ public Builder credentialsProvider(Provider provider) { * Supplies a custom {@link OkHttpClient}. The SDK wraps this client to install {@link * Http.RetryInterceptor} and forces {@code retryOnConnectionFailure(false)} so that {@link * Http.RetryInterceptor} owns the entire retry policy. Any prior {@code RetryInterceptor} on - * the supplied client is replaced. + * either the application-interceptor or network-interceptor chain of the supplied client is + * stripped before the new one is installed (as an application interceptor). */ public Builder httpClient(OkHttpClient httpClient) { Utils.validateNotNull(httpClient, "http client"); @@ -231,7 +232,7 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) { /** * Sets the maximum number of attempts per request. Pass {@code 1} to disable automatic retries. - * Defaults to {@link Retry#MAX_RETRY}. + * Defaults to {@code 10}. */ public Builder maxRetries(int maxRetries) { if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1"); diff --git a/api/src/main/java/io/minio/Retry.java b/api/src/main/java/io/minio/Retry.java index 9d6a8b56b..68f781d9b 100644 --- a/api/src/main/java/io/minio/Retry.java +++ b/api/src/main/java/io/minio/Retry.java @@ -114,11 +114,15 @@ static boolean isRequestErrorRetryable(IOException e) { * *

    *   sleep = min(DEFAULT_RETRY_CAP_MS, DEFAULT_RETRY_UNIT_MS * 2^attempt)
-   *   sleep -= random.nextDouble() * sleep * MAX_JITTER     // full jitter when MAX_JITTER == 1.0
+   *   sleep -= (long)(random.nextDouble() * sleep * MAX_JITTER)   // full jitter when MAX_JITTER == 1.0
    * 
* - *

With {@code MAX_JITTER == 1.0}, returns a uniform random value in {@code [0, min(cap, base * - * 2^attempt)]}. + *

With {@code MAX_JITTER == 1.0}, returns a value in {@code [1, min(cap, base * 2^attempt)]}. + * The lower bound is {@code 1} rather than {@code 0} because {@link + * java.util.concurrent.ThreadLocalRandom#nextDouble()} is in {@code [0.0, 1.0)} and the {@code + * (long)} cast truncates {@code rand * sleep} to at most {@code sleep - 1}. This matches the + * behaviour of minio-go's {@code exponentialBackoffWait}, which uses the same formula and + * therefore the same bounds. */ static long exponentialBackoffMs(int attempt) { int exp = Math.min(Math.max(attempt, 0), 30); From 9a5c8e4f979996d339fbfc06ba4f3d94b111bc70 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Fri, 8 May 2026 05:29:20 +0000 Subject: [PATCH 15/15] fix(retry): address bala review on PR #1701 review 4248622939 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five concessions from bala's review batch — each verified against minio-go's contract or the file's own pre-PR conventions: BaseS3Client.java - Restore the `protected static final Random RANDOM` field (with its java.security.SecureRandom + java.util.Random imports) that 60aeaed4 dropped when switching backoff jitter to ThreadLocalRandom. Per bala's r3205617158: do not remove protected/public API surface. The field is part of the public subclassing contract on BaseS3Client and is the Java analogue of minio-go's per-client `c.random` source (api.go:307); SDK-internal code paths still use ThreadLocalRandom.current() for the retry backoff hot path so jitter is unchanged. - Drop the null-guard around `regionCache.remove(s3request.bucket())` introduced in beead6e76. The guard was only load-bearing for the contrived testNoRetryOn404 (listBuckets + 404 NoSuchBucket — a wire shape no spec-compliant S3 server emits since NoSuchBucket only applies to bucket-scoped ops); the test is rewritten in this commit to a realistic scenario, removing the dependency on the guard. Http.java - Drop the two-line `// Our RetryInterceptor handles transient failures with full-jitter backoff; defer entirely to it instead of layering OkHttp's connection-level retry on top.` comment from newDefaultClient() per bala's r3205589355: the retryOnConnectionFailure(false) call paired with addInterceptor(new RetryInterceptor()) is self-explanatory and the prose duplicated the RetryInterceptor Javadoc. - Drop the `Utils.validateNotNull(requestBody, "request body")` guard from the Body(okhttp3.RequestBody) constructor per bala's r3205613399; the three other Body constructors do not validate non-null and let the next-line dereference fail with the JVM's helpful-NPE message — restoring that local convention. - Drop the `private long fileOffset` field, its capture in the Body(RandomAccessFile,...) constructor, and the seek in toRequestBody() per bala's r3205614300. Empirical truth-table probe (4 trials toggling F = Body.fileOffset against C+R = createBody save/restore + RequestBody.position+seek) confirmed F is redundant when C+R are present: removing F alone keeps both first-attempt and retry-attempt body content correct (Trial 3), since the pre-Body-construction seek in createBody already places the file pointer where RequestBody.position will capture it on construction and writeTo() will seek back to on each network attempt. RetryTest.java - Rewrite testNoRetryOn404 to call removeBucket on a non-existent bucket. NoSuchBucket only applies to bucket-scoped operations on the S3 wire; pairing it with the bucket-less listBuckets() is a contrived scenario AWS S3 / MinIO never produce. The rewrite preserves coverage (404 does not retry, ErrorResponseException carries code "NoSuchBucket" and status 404, request count == 1) while exercising a wire shape the SDK actually has to handle, and uses a non-null bucket so the regionCache invalidation path runs with a real ConcurrentHashMap.remove(String) call. Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - :api:javadoc PASS --- api/src/main/java/io/minio/BaseS3Client.java | 10 ++++++---- api/src/main/java/io/minio/Http.java | 19 ++----------------- api/src/test/java/io/minio/RetryTest.java | 6 +++++- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/api/src/main/java/io/minio/BaseS3Client.java b/api/src/main/java/io/minio/BaseS3Client.java index f4cc5d115..2d0e7377a 100644 --- a/api/src/main/java/io/minio/BaseS3Client.java +++ b/api/src/main/java/io/minio/BaseS3Client.java @@ -51,11 +51,13 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Locale; import java.util.Map; +import java.util.Random; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -90,6 +92,7 @@ public abstract class BaseS3Client implements AutoCloseable { "ServerSideEncryptionConfigurationNotFoundError"; // maximum allowed bucket policy size is 20KiB protected static final int MAX_BUCKET_POLICY_SIZE = 20 * 1024; + protected static final Random RANDOM = new Random(new SecureRandom().nextLong()); protected static final ObjectMapper OBJECT_MAPPER = JsonMapper.builder() .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) @@ -512,10 +515,9 @@ private void onResponse(final Response response) throws IOException { response.header("x-amz-id-2")); } - // invalidate region cache if needed (bucket may be null for e.g. listBuckets) - if (s3request.bucket() != null - && (errorResponse.code().equals(NO_SUCH_BUCKET) - || errorResponse.code().equals(RETRY_HEAD))) { + // invalidate region cache if needed + if (errorResponse.code().equals(NO_SUCH_BUCKET) + || errorResponse.code().equals(RETRY_HEAD)) { regionCache.remove(s3request.bucket()); } diff --git a/api/src/main/java/io/minio/Http.java b/api/src/main/java/io/minio/Http.java index b33d9c013..d667bedbc 100644 --- a/api/src/main/java/io/minio/Http.java +++ b/api/src/main/java/io/minio/Http.java @@ -623,8 +623,6 @@ public static OkHttpClient newDefaultClient() { .writeTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS) .readTimeout(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS) .protocols(Arrays.asList(Protocol.HTTP_1_1)) - // Our RetryInterceptor handles transient failures with full-jitter backoff; defer - // entirely to it instead of layering OkHttp's connection-level retry on top. .retryOnConnectionFailure(false) .addInterceptor(new RetryInterceptor()) .build(); @@ -846,7 +844,6 @@ private static String peekS3ErrorCode(okhttp3.Response response) { public static class Body { private okhttp3.RequestBody requestBody; private RandomAccessFile file; - private long fileOffset; private ByteBuffer buffer; private byte[] data; private Long length; @@ -864,7 +861,7 @@ public static class Body { * disabled for that call. */ public Body(okhttp3.RequestBody requestBody) { - this.requestBody = Utils.validateNotNull(requestBody, "request body"); + this.requestBody = requestBody; this.contentType = requestBody.contentType(); } @@ -877,11 +874,6 @@ public Body( String md5Hash) { if (length < 0) throw new IllegalArgumentException("valid length must be provided"); this.file = file; - try { - this.fileOffset = file.getFilePointer(); - } catch (IOException e) { - throw new IllegalStateException("failed to read file position", e); - } set(length, contentType, sha256Hash, md5Hash); } @@ -955,14 +947,7 @@ public Headers headers() { /** Creates HTTP RequestBody for this body. */ public RequestBody toRequestBody() throws MinioException { if (requestBody != null) return new RequestBody(requestBody); - if (file != null) { - try { - file.seek(fileOffset); - } catch (IOException e) { - throw new MinioException(e); - } - return new RequestBody(file, length, contentType); - } + if (file != null) return new RequestBody(file, length, contentType); if (buffer != null) return new RequestBody(buffer, contentType); return new RequestBody(data, length.intValue(), contentType); } diff --git a/api/src/test/java/io/minio/RetryTest.java b/api/src/test/java/io/minio/RetryTest.java index e5b508b57..cd7a58213 100644 --- a/api/src/test/java/io/minio/RetryTest.java +++ b/api/src/test/java/io/minio/RetryTest.java @@ -282,6 +282,9 @@ public void testRetryOnRetryableS3CodeIn400Body() throws IOException, MinioExcep @Test public void testNoRetryOn404() throws IOException, MinioException { + // Bucket-scoped operation returning 404 NoSuchBucket — a wire-realistic shape + // (NoSuchBucket only ever applies to bucket-scoped calls). Verifies that 404 + // does not trigger retry and that the parsed error surfaces both code and status. try (MockWebServer server = new MockWebServer()) { server.enqueue(xmlError(404, "NoSuchBucket")); server.start(); @@ -290,7 +293,8 @@ public void testNoRetryOn404() throws IOException, MinioException { MinioClient.builder().endpoint(server.url("").toString()).maxRetries(3).build(); try { try { - client.listBuckets(); + client.removeBucket( + RemoveBucketArgs.builder().bucket("missing-bucket-for-no-retry").build()); Assert.fail("expected ErrorResponseException"); } catch (ErrorResponseException e) { Assert.assertEquals("NoSuchBucket", e.errorResponse().code());