Skip to content

Commit 7cf9410

Browse files
Copilotbaywet
andcommitted
refactor: move negative retry interval check to getRetryAfter method
Move the negative value handling from retryRequest to getRetryAfter method as suggested in code review. This ensures getRetryAfter never returns a negative value, making the API more robust. - Move check from retryRequest to getRetryAfter - Update test to verify getRetryAfter returns 1-10ms instead of -1 - Maintain same behavior: random delay between 1-10ms for negative values Co-authored-by: baywet <[email protected]>
1 parent f79bc62 commit 7cf9410

File tree

2 files changed

+12
-8
lines changed

2 files changed

+12
-8
lines changed

components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RetryHandler.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,6 @@ && isBuffered(request)
110110

111111
if (shouldRetry) {
112112
long retryInterval = getRetryAfter(response, retryOption.delay(), executionCount);
113-
// Ensure minimum delay if retry interval is negative
114-
if (retryInterval < 0) {
115-
retryInterval = 1 + (long) (Math.random() * 9); // Random delay between 1-10ms
116-
}
117113
span.setAttribute(HTTP_REQUEST_RESEND_DELAY, Math.round(retryInterval / 1000f));
118114
try {
119115
Thread.sleep(retryInterval);
@@ -142,7 +138,13 @@ long getRetryAfter(Response response, long delay, int executionCount) {
142138
} else if (retryDelay == -1) {
143139
retryDelay = exponentialBackOffDelay(delay, executionCount);
144140
}
145-
return (long) Math.min(retryDelay, RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS);
141+
long result =
142+
(long) Math.min(retryDelay, RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS);
143+
// Ensure minimum delay if retry interval is negative
144+
if (result < 0) {
145+
result = 1 + (long) (Math.random() * 9); // Random delay between 1-10ms
146+
}
147+
return result;
146148
}
147149

148150
/**

components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RetryHandlerTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,17 @@ void testTryParseTimeHeader(String headerValue, double expectedDelay) {
7171
}
7272

7373
@Test
74-
void testGetRetryAfterReturnsNegativeWhenHeaderParsingFails() {
74+
void testGetRetryAfterHandlesNegativeValues() {
7575
// Create a mock response with a Retry-After header that will fail parsing
7676
Response mockResponse = mock(Response.class);
7777
when(mockResponse.header("Retry-After")).thenReturn("invalid");
7878
when(mockResponse.code()).thenReturn(429);
7979

80-
// getRetryAfter should return -1 when parsing fails but header exists
80+
// getRetryAfter should return a value between 1-10ms when parsing fails
8181
long retryAfter = retryHandler.getRetryAfter(mockResponse, 3, 1);
82-
assertEquals(-1, retryAfter, "Expected -1 when Retry-After header parsing fails");
82+
assertTrue(
83+
retryAfter >= 1 && retryAfter <= 10,
84+
"Expected retry interval between 1-10ms, got " + retryAfter);
8385
}
8486

8587
@Test

0 commit comments

Comments
 (0)