Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -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
}
Comment thread
baywet marked this conversation as resolved.
Outdated
span.setAttribute(HTTP_REQUEST_RESEND_DELAY, Math.round(retryInterval / 1000f));
try {
Thread.sleep(retryInterval);
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,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 {
Comment thread
baywet marked this conversation as resolved.
Outdated
// 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 {
Comment thread
baywet marked this conversation as resolved.
Outdated
// 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);
}
}
}