Skip to content

Commit 1defea4

Browse files
committed
review: fix hot-loop scenario
1 parent 153c570 commit 1defea4

File tree

2 files changed

+50
-2
lines changed

2 files changed

+50
-2
lines changed

src/main/java/dev/openfga/sdk/util/RetryStrategy.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,12 @@ public static boolean shouldRetry(int statusCode) {
6666
*/
6767
public static Duration calculateRetryDelay(
6868
Optional<Duration> retryAfterDelay, int retryCount, Duration minimumRetryDelay) {
69-
// If Retry-After header is present, use it (takes precedence over minimum retry delay)
69+
// If Retry-After header is present, use it but enforce minimum delay floor
7070
if (retryAfterDelay.isPresent()) {
71-
return retryAfterDelay.get();
71+
Duration serverDelay = retryAfterDelay.get();
72+
// Clamp to minimum 1ms to prevent hot-loop retries and handle malformed server responses
73+
Duration minimumSafeDelay = Duration.ofMillis(1);
74+
return serverDelay.compareTo(minimumSafeDelay) < 0 ? minimumSafeDelay : serverDelay;
7275
}
7376

7477
// Otherwise, use exponential backoff with jitter, respecting minimum retry delay

src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,49 @@ void shouldRetry_with404_shouldReturnFalse() {
120120
void shouldRetry_with501_shouldReturnFalse() {
121121
assertThat(RetryStrategy.shouldRetry(501)).isFalse();
122122
}
123+
124+
@Test
125+
void calculateRetryDelay_withZeroRetryAfter_shouldEnforceMinimumDelay() {
126+
// Given - Server sends Retry-After: 0 (problematic!)
127+
Optional<Duration> retryAfterDelay = Optional.of(Duration.ZERO);
128+
int retryCount = 1;
129+
Duration minimumRetryDelay = Duration.ofMillis(100);
130+
131+
// When
132+
Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay);
133+
134+
// Then - Should enforce minimum delay to prevent hot-loop retries
135+
// Current code will FAIL this test by returning Duration.ZERO
136+
assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms to prevent hot-loops
137+
}
138+
139+
@Test
140+
void calculateRetryDelay_withNegativeRetryAfter_shouldEnforceMinimumDelay() {
141+
// Given - Server sends malformed negative delay (very problematic!)
142+
Optional<Duration> retryAfterDelay = Optional.of(Duration.ofMillis(-500));
143+
int retryCount = 1;
144+
Duration minimumRetryDelay = Duration.ofMillis(100);
145+
146+
// When
147+
Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay);
148+
149+
// Then - Should enforce minimum delay to handle malformed server responses
150+
// Current code will FAIL this test by returning negative duration
151+
assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms for safety
152+
}
153+
154+
@Test
155+
void calculateRetryDelay_withVerySmallRetryAfter_shouldEnforceMinimumDelay() {
156+
// Given - Server sends extremely small delay (could cause near-hot-loop)
157+
Optional<Duration> retryAfterDelay = Optional.of(Duration.ofNanos(500)); // 0.0005ms
158+
int retryCount = 1;
159+
Duration minimumRetryDelay = Duration.ofMillis(100);
160+
161+
// When
162+
Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay);
163+
164+
// Then - Should enforce reasonable minimum delay
165+
// Current code will FAIL this test by returning tiny delay
166+
assertThat(result.toMillis()).isGreaterThanOrEqualTo(1); // At least 1ms for system stability
167+
}
123168
}

0 commit comments

Comments
 (0)