Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
}
}