Merge pull request #2051 from microsoft/copilot/fix-retry-handler-timeout

fix: prevent negative timeout in RetryHandler causing IllegalArgumentException

Author: baywet

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

@@ -59,7 +59,7 @@ public class RetryHandler implements Interceptor {
     /**
      * One second as milliseconds
      */
-    private static final long DELAY_MILLISECONDS = 1000;
+    private static final int DELAY_MILLISECONDS = 1000;
 
     /**
      * Initialize retry handler with retry option
@@ -138,7 +138,16 @@ long getRetryAfter(Response response, long delay, int executionCount) {
         } else if (retryDelay == -1) {
             retryDelay = exponentialBackOffDelay(delay, executionCount);
         }
-        return (long) Math.min(retryDelay, RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS);
+        long result =
+                (long)
+                        Math.min(
+                                retryDelay,
+                                (double) RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS);
+        // Ensure minimum delay if retry interval is negative
+        if (result < 0) {
+            result = 1 + (long) (Math.random() * 9); // Random delay between 1-10ms
+        }
+        return result;
     }
 
     /**

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,75 @@ void testTryParseTimeHeader(String headerValue, double expectedDelay) {
         assertEquals(
                 expectedDelay, result, DELTA, "Failed for header value: '" + headerValue + "'");
     }
+
+    @Test
+    void testGetRetryAfterHandlesNegativeValues() {
+        // 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 a value between 1-10ms when parsing fails
+        long retryAfter = retryHandler.getRetryAfter(mockResponse, 3, 1);
+        assertTrue(
+                retryAfter >= 1 && retryAfter <= 10,
+                "Expected retry interval between 1-10ms, got " + retryAfter);
+    }
+
+    @Test
+    void testRetryRequestHandlesNegativeRetryInterval() {
+        // 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() {
+        // 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);
+        }
+    }
 }