From a903113f5d5b6e8818be373949716e5e0c70c7b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:10:10 +0000 Subject: [PATCH 1/8] Initial plan From f79bc626b2c51efa06fc7eef5f0b9ff2ee1e5048 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:21:21 +0000 Subject: [PATCH 2/8] fix: handle negative retry interval in RetryHandler - 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 <7905502+baywet@users.noreply.github.com> --- .../kiota/http/middleware/RetryHandler.java | 4 + .../http/middleware/RetryHandlerTest.java | 80 +++++++++++++++++++ 2 files changed, 84 insertions(+) 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..23d979c1 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 @@ -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 index 41ad313d..d4b7cea6 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,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); + } + } } From 7cf9410176e6eb3dcb604d17cd4abcb541100be5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:44:19 +0000 Subject: [PATCH 3/8] 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 <7905502+baywet@users.noreply.github.com> --- .../kiota/http/middleware/RetryHandler.java | 12 +++++++----- .../kiota/http/middleware/RetryHandlerTest.java | 8 +++++--- 2 files changed, 12 insertions(+), 8 deletions(-) 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 23d979c1..e881d056 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 @@ -110,10 +110,6 @@ && 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); @@ -142,7 +138,13 @@ 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, 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 d4b7cea6..54f811c6 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 @@ -71,15 +71,17 @@ void testTryParseTimeHeader(String headerValue, double expectedDelay) { } @Test - void testGetRetryAfterReturnsNegativeWhenHeaderParsingFails() { + 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 -1 when parsing fails but header exists + // getRetryAfter should return a value between 1-10ms when parsing fails long retryAfter = retryHandler.getRetryAfter(mockResponse, 3, 1); - assertEquals(-1, retryAfter, "Expected -1 when Retry-After header parsing fails"); + assertTrue( + retryAfter >= 1 && retryAfter <= 10, + "Expected retry interval between 1-10ms, got " + retryAfter); } @Test From f9c23ee5c3a25c68de43a16f0fec162b95982cc4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:54:18 +0000 Subject: [PATCH 4/8] refactor: use Random.nextLong() instead of Math.random() for delay Replace Math.random() with Random.nextLong(10) for generating random delays between 1-10ms. This provides better control and clarity over the random value generation. - Add java.util.Random import - Create static RANDOM instance - Use RANDOM.nextLong(10) to generate values 0-9, then add 1 for 1-10ms range Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --- .../com/microsoft/kiota/http/middleware/RetryHandler.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 e881d056..64b40cd8 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 @@ -23,6 +23,7 @@ import java.time.format.DateTimeParseException; import java.time.temporal.ChronoUnit; import java.util.Objects; +import java.util.Random; /** * The middleware responsible for retrying requests when they fail because of transient issues @@ -61,6 +62,11 @@ public class RetryHandler implements Interceptor { */ private static final long DELAY_MILLISECONDS = 1000; + /** + * Random instance for generating random delays + */ + private static final Random RANDOM = new Random(); + /** * Initialize retry handler with retry option * @param retryOption Create Retry handler using retry option @@ -142,7 +148,7 @@ long getRetryAfter(Response response, long delay, int executionCount) { (long) Math.min(retryDelay, 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 + result = 1 + RANDOM.nextLong(10); // Random delay between 1-10ms } return result; } From f21b4139d7a2181c42c94fab12f00593044f2a24 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 14 Nov 2025 10:03:46 -0500 Subject: [PATCH 5/8] Apply suggestions from code review --- .../com/microsoft/kiota/http/middleware/RetryHandlerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 54f811c6..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 @@ -85,7 +85,7 @@ void testGetRetryAfterHandlesNegativeValues() { } @Test - void testRetryRequestHandlesNegativeRetryInterval() throws Exception { + void testRetryRequestHandlesNegativeRetryInterval() { // Create mocks Response mockResponse = mock(Response.class); Request mockRequest = mock(Request.class); @@ -114,7 +114,7 @@ void testRetryRequestHandlesNegativeRetryInterval() throws Exception { } @Test - void testRetryRequestWithNegativeRetryIntervalAppliesRandomDelay() throws Exception { + void testRetryRequestWithNegativeRetryIntervalAppliesRandomDelay() { // Create mocks Response mockResponse = mock(Response.class); Request mockRequest = mock(Request.class); From ea5d578db8e8595cd2ae46735be2e55cebaf4edd Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 14 Nov 2025 10:18:15 -0500 Subject: [PATCH 6/8] Revert "refactor: use Random.nextLong() instead of Math.random() for delay" This reverts commit f9c23ee5c3a25c68de43a16f0fec162b95982cc4. --- .../com/microsoft/kiota/http/middleware/RetryHandler.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) 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 64b40cd8..e881d056 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 @@ -23,7 +23,6 @@ import java.time.format.DateTimeParseException; import java.time.temporal.ChronoUnit; import java.util.Objects; -import java.util.Random; /** * The middleware responsible for retrying requests when they fail because of transient issues @@ -62,11 +61,6 @@ public class RetryHandler implements Interceptor { */ private static final long DELAY_MILLISECONDS = 1000; - /** - * Random instance for generating random delays - */ - private static final Random RANDOM = new Random(); - /** * Initialize retry handler with retry option * @param retryOption Create Retry handler using retry option @@ -148,7 +142,7 @@ long getRetryAfter(Response response, long delay, int executionCount) { (long) Math.min(retryDelay, RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS); // Ensure minimum delay if retry interval is negative if (result < 0) { - result = 1 + RANDOM.nextLong(10); // Random delay between 1-10ms + result = 1 + (long) (Math.random() * 9); // Random delay between 1-10ms } return result; } From a6227ff7108f4d4317f501cdb11e33b2f9dd9302 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 14 Nov 2025 10:28:38 -0500 Subject: [PATCH 7/8] chore: applies sonarqube recommendation Signed-off-by: Vincent Biret --- .../com/microsoft/kiota/http/middleware/RetryHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e881d056..3b8fe9d1 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 @@ -139,7 +139,7 @@ long getRetryAfter(Response response, long delay, int executionCount) { retryDelay = exponentialBackOffDelay(delay, executionCount); } long result = - (long) Math.min(retryDelay, RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS); + (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 From 57c6753906c9c5bcdf539e1a4dd891dbaaa1020c Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Fri, 14 Nov 2025 10:33:22 -0500 Subject: [PATCH 8/8] chore: formatting --- .../com/microsoft/kiota/http/middleware/RetryHandler.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 3b8fe9d1..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 @@ -139,7 +139,10 @@ long getRetryAfter(Response response, long delay, int executionCount) { retryDelay = exponentialBackOffDelay(delay, executionCount); } long result = - (long) Math.min(retryDelay, (double)RetryHandlerOption.MAX_DELAY * DELAY_MILLISECONDS); + (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