From 564a1f0d52a6c2bc2e39974ff699b77173ca824d Mon Sep 17 00:00:00 2001 From: Will Howes Date: Wed, 20 May 2026 18:08:29 +0000 Subject: [PATCH 1/5] fix(gax): adjust RetryAlgorithm logic for timeout v. exception --- .../com/google/api/gax/retrying/RetryAlgorithm.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java index 16d60d148d38..accb9fd0dc15 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java @@ -232,8 +232,15 @@ public boolean shouldRetry( ResponseT previousResponse, TimedAttemptSettings nextAttemptSettings) throws CancellationException { - return shouldRetryBasedOnResult(context, previousThrowable, previousResponse) - && shouldRetryBasedOnTiming(context, nextAttemptSettings); + boolean retryBasedOnResult = shouldRetryBasedOnResult(context, previousThrowable, previousResponse); + + // Short-circuit when operation has succeeded to avoid erroneously throwing CancellationException below + if (!retryBasedOnResult && previousThrowable == null) { + return false; + } + + boolean retryBasedOnTiming = shouldRetryBasedOnTiming(context, nextAttemptSettings); // throws CancellationException + return retryBasedOnResult && retryBasedOnTiming; } boolean shouldRetryBasedOnResult( From 9647336450341d3874aaf408e216cb6b40c2c5e8 Mon Sep 17 00:00:00 2001 From: Will Howes Date: Wed, 20 May 2026 19:27:47 +0000 Subject: [PATCH 2/5] test(gax): add tests for RetryAlgorithm changes --- .../api/gax/retrying/RetryAlgorithmTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java index fb9e2f17f308..8eb0e9f39e15 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java @@ -31,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -201,4 +202,34 @@ void testShouldRetryWithContext_noPreviousSettings() { assertFalse(algorithm.shouldRetry(context, previousThrowable, previousResult, null)); } + + @Test + void testShouldRetry_prevThrowableNotNull_shouldRetryBasedOnResultFalse_callsTimedAlgorithm() { + ResultRetryAlgorithm resultAlgorithm = mock(ResultRetryAlgorithm.class); + TimedRetryAlgorithm timedAlgorithm = mock(TimedRetryAlgorithm.class); + RetryAlgorithm algorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); + Throwable previousThrowable = new Throwable(); + Object previousResult = new Object(); + TimedAttemptSettings previousSettings = mock(TimedAttemptSettings.class); + when(resultAlgorithm.shouldRetry(previousThrowable, previousResult)).thenReturn(false); + + algorithm.shouldRetry(previousThrowable, previousResult, previousSettings); + + verify(timedAlgorithm).shouldRetry(previousSettings); + } + + @Test + void testShouldRetry_prevThrowableNull_shouldRetryBasedOnResultFalse_shortCircuits() { + ResultRetryAlgorithm resultAlgorithm = mock(ResultRetryAlgorithm.class); + TimedRetryAlgorithm timedAlgorithm = mock(TimedRetryAlgorithm.class); + RetryAlgorithm algorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); + Object previousResult = new Object(); + TimedAttemptSettings previousSettings = mock(TimedAttemptSettings.class); + when(resultAlgorithm.shouldRetry(null, previousResult)).thenReturn(false); + + boolean shouldRetry = algorithm.shouldRetry(null, previousResult, previousSettings); + + assertFalse(shouldRetry); + verify(timedAlgorithm, never()).shouldRetry(previousSettings); + } } From f64895094f4c48cd16e9ebab98d5e1cce4a7490e Mon Sep 17 00:00:00 2001 From: Will Howes Date: Wed, 20 May 2026 20:31:59 +0000 Subject: [PATCH 3/5] fix(gax): fix linter issues --- .../java/com/google/api/gax/retrying/RetryAlgorithm.java | 9 ++++++--- .../com/google/api/gax/retrying/RetryAlgorithmTest.java | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java index accb9fd0dc15..5fd3674f37c9 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java @@ -232,14 +232,17 @@ public boolean shouldRetry( ResponseT previousResponse, TimedAttemptSettings nextAttemptSettings) throws CancellationException { - boolean retryBasedOnResult = shouldRetryBasedOnResult(context, previousThrowable, previousResponse); + boolean retryBasedOnResult = + shouldRetryBasedOnResult(context, previousThrowable, previousResponse); - // Short-circuit when operation has succeeded to avoid erroneously throwing CancellationException below + // Short-circuit when operation has succeeded to avoid erroneously throwing + // CancellationException below if (!retryBasedOnResult && previousThrowable == null) { return false; } - boolean retryBasedOnTiming = shouldRetryBasedOnTiming(context, nextAttemptSettings); // throws CancellationException + // throws CancellationException + boolean retryBasedOnTiming = shouldRetryBasedOnTiming(context, nextAttemptSettings); return retryBasedOnResult && retryBasedOnTiming; } diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java index 8eb0e9f39e15..2fdef08eebab 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java @@ -204,7 +204,7 @@ void testShouldRetryWithContext_noPreviousSettings() { } @Test - void testShouldRetry_prevThrowableNotNull_shouldRetryBasedOnResultFalse_callsTimedAlgorithm() { + void testShouldRetry_resultFalseAndThrowableNotNull_callsTimed() { ResultRetryAlgorithm resultAlgorithm = mock(ResultRetryAlgorithm.class); TimedRetryAlgorithm timedAlgorithm = mock(TimedRetryAlgorithm.class); RetryAlgorithm algorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); @@ -219,7 +219,7 @@ void testShouldRetry_prevThrowableNotNull_shouldRetryBasedOnResultFalse_callsTim } @Test - void testShouldRetry_prevThrowableNull_shouldRetryBasedOnResultFalse_shortCircuits() { + void testShouldRetry_resultFalseAndThrowableNull_shortCircuits() { ResultRetryAlgorithm resultAlgorithm = mock(ResultRetryAlgorithm.class); TimedRetryAlgorithm timedAlgorithm = mock(TimedRetryAlgorithm.class); RetryAlgorithm algorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); From 54b9310abbb3fbb1adb9633f67241548d50d3be2 Mon Sep 17 00:00:00 2001 From: Will Howes Date: Wed, 20 May 2026 20:36:30 +0000 Subject: [PATCH 4/5] fix(gax): add assertion to test --- .../java/com/google/api/gax/retrying/RetryAlgorithmTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java index 2fdef08eebab..2121bf816c84 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/retrying/RetryAlgorithmTest.java @@ -213,8 +213,10 @@ void testShouldRetry_resultFalseAndThrowableNotNull_callsTimed() { TimedAttemptSettings previousSettings = mock(TimedAttemptSettings.class); when(resultAlgorithm.shouldRetry(previousThrowable, previousResult)).thenReturn(false); - algorithm.shouldRetry(previousThrowable, previousResult, previousSettings); + boolean shouldRetry = + algorithm.shouldRetry(previousThrowable, previousResult, previousSettings); + assertFalse(shouldRetry); verify(timedAlgorithm).shouldRetry(previousSettings); } From 239df6ab3f1cef962e1f54690b946700e4d78d89 Mon Sep 17 00:00:00 2001 From: Will Howes Date: Wed, 20 May 2026 17:27:51 -0700 Subject: [PATCH 5/5] clarify comment in RetryAlgorithm --- .../main/java/com/google/api/gax/retrying/RetryAlgorithm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java index 5fd3674f37c9..d945e6d0533f 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/retrying/RetryAlgorithm.java @@ -241,7 +241,7 @@ public boolean shouldRetry( return false; } - // throws CancellationException + // may throw CancellationException if timeout is saturated boolean retryBasedOnTiming = shouldRetryBasedOnTiming(context, nextAttemptSettings); return retryBasedOnResult && retryBasedOnTiming; }