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 index 81d34562..275acdb8 100644 --- 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 @@ -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; } /** 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 index 41ad313d..747252ba 100644 --- 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,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); + } + } }