fix: handle negative retry interval in RetryHandler

Copilot · baywet · Copilot · commit f79bc626b2c5 · 2025-11-14T14:21:21.000Z
- Add check for negative retry interval in retryRequest method
- Set random delay between 1-10ms when retry interval is negative
- Add unit tests for negative retry interval scenarios
- Prevents IllegalArgumentException: timeout value is negative

Co-authored-by: baywet &lt;7905502+baywet@users.noreply.github.com&gt;
diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RetryHandler.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RetryHandler.java
@@ -110,6 +110,10 @@ && isBuffered(request)
 
         if (shouldRetry) {
             long retryInterval = getRetryAfter(response, retryOption.delay(), executionCount);
+            // Ensure minimum delay if retry interval is negative
+            if (retryInterval < 0) {
+                retryInterval = 1 + (long) (Math.random() * 9); // Random delay between 1-10ms
+            }
             span.setAttribute(HTTP_REQUEST_RESEND_DELAY, Math.round(retryInterval / 1000f));
             try {
                 Thread.sleep(retryInterval);
diff --git a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RetryHandlerTest.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RetryHandlerTest.java
@@ -1,8 +1,19 @@
 package com.microsoft.kiota.http.middleware;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.microsoft.kiota.http.middleware.options.RetryHandlerOption;
+
+import io.opentelemetry.api.trace.Span;
+
+import okhttp3.Request;
+import okhttp3.Response;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -58,4 +69,73 @@ void testTryParseTimeHeader(String headerValue, double expectedDelay) {
         assertEquals(
                 expectedDelay, result, DELTA, "Failed for header value: '" + headerValue + "'");
     }
+
+    @Test
+    void testGetRetryAfterReturnsNegativeWhenHeaderParsingFails() {
+        // Create a mock response with a Retry-After header that will fail parsing
+        Response mockResponse = mock(Response.class);
+        when(mockResponse.header("Retry-After")).thenReturn("invalid");
+        when(mockResponse.code()).thenReturn(429);
+
+        // getRetryAfter should return -1 when parsing fails but header exists
+        long retryAfter = retryHandler.getRetryAfter(mockResponse, 3, 1);
+        assertEquals(-1, retryAfter, "Expected -1 when Retry-After header parsing fails");
+    }
+
+    @Test
+    void testRetryRequestHandlesNegativeRetryInterval() throws Exception {
+        // Create mocks
+        Response mockResponse = mock(Response.class);
+        Request mockRequest = mock(Request.class);
+        RetryHandlerOption mockOption = mock(RetryHandlerOption.class);
+        Span mockSpan = mock(Span.class);
+
+        // Setup mock behaviors
+        when(mockResponse.code()).thenReturn(429);
+        when(mockResponse.header("Retry-After")).thenReturn("invalid");
+        when(mockRequest.method()).thenReturn("GET");
+        when(mockRequest.body()).thenReturn(null);
+        when(mockOption.maxRetries()).thenReturn(3);
+        when(mockOption.delay()).thenReturn(3L);
+        when(mockOption.shouldRetry())
+                .thenReturn(
+                        (delay, executionCount, request, response) ->
+                                true); // Always retry for this test
+
+        // Call retryRequest - it should not throw IllegalArgumentException
+        // The main goal is to verify no exception is thrown when retry interval is negative
+        boolean result =
+                retryHandler.retryRequest(mockResponse, 1, mockRequest, mockOption, mockSpan);
+
+        // Verify the method returned true (should retry)
+        assertTrue(result, "Expected retryRequest to return true");
+    }
+
+    @Test
+    void testRetryRequestWithNegativeRetryIntervalAppliesRandomDelay() throws Exception {
+        // Create mocks
+        Response mockResponse = mock(Response.class);
+        Request mockRequest = mock(Request.class);
+        RetryHandlerOption mockOption = mock(RetryHandlerOption.class);
+        Span mockSpan = mock(Span.class);
+
+        // Setup mock behaviors to force negative retry interval
+        when(mockResponse.code()).thenReturn(503);
+        when(mockResponse.header("Retry-After")).thenReturn("invalid");
+        when(mockRequest.method()).thenReturn("POST");
+        when(mockRequest.body()).thenReturn(null);
+        when(mockOption.maxRetries()).thenReturn(5);
+        when(mockOption.delay()).thenReturn(1L);
+        when(mockOption.shouldRetry())
+                .thenReturn((delay, executionCount, request, response) -> true);
+
+        // Test multiple times to verify no exception is thrown
+        // This verifies the fix for negative retry intervals
+        for (int i = 0; i < 5; i++) {
+            boolean result =
+                    retryHandler.retryRequest(mockResponse, 1, mockRequest, mockOption, mockSpan);
+            // Verify each call succeeds without throwing IllegalArgumentException
+            assertTrue(result, "Expected retryRequest to return true on iteration " + i);
+        }
+    }
 }
