Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 14, 2025

Fix RetryHandler negative timeout issue

Plan

  • Understand the issue: getRetryAfter can return negative value causing IllegalArgumentException
  • Analyze existing code and tests
  • Fix getRetryAfter to ensure minimum delay of 1-10ms when result is negative
  • Add unit tests for negative retry interval case
  • Run tests to verify the fix
  • Verify code style with spotless
  • Run build to ensure no regressions
  • Run security check (CodeQL) - No vulnerabilities found
  • Address code review feedback: Move check to getRetryAfter method
  • Address code review feedback: Use Random.nextLong() instead of Math.random()

Problem

The RetryHandler.retryRequest method can fail with IllegalArgumentException: timeout value is negative when getRetryAfter returns a negative value. This happens when:

  1. Retry-After header exists but parsing fails
  2. Date header parsing returns a date in the past

Solution

Set a minimum random delay between 1-10ms in the getRetryAfter method when the calculated value is less than 0, as specified in the issue requirements.

Changes Made

  1. RetryHandler.java (lines 147-151): Handle negative interval in getRetryAfter method using Random.nextLong()
  2. RetryHandlerTest.java: Updated test to verify getRetryAfter returns 1-10ms instead of -1 when parsing fails

Verification

  • ✅ All 20 unit tests in RetryHandlerTest pass
  • ✅ Code formatting verified with spotless
  • ✅ Module builds successfully
  • ✅ No security vulnerabilities detected by CodeQL

Code Review

  • ✅ Addressed feedback from @baywet to move check to getRetryAfter method
  • ✅ Addressed feedback from @baywet to use Random.nextLong() instead of Math.random()
Original prompt

This section details on the original issue you should resolve

<issue_title>RetryHandler#retryRequest might fail with IllegalArgumentException: timeout value is negative</issue_title>
<issue_description>When using the Microsoft Graph API which then calls com.microsoft.kiota.http.middleware.RetryHandler.retryRequest (see stack trace below), it seems to trigger retryRequest with a negative retryInterval value. It seems that getRetryAfter is returning a negative value at https://github.com/microsoft/kiota-java/blob/main/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RetryHandler.java#L112.

Please see stack trace:

java.lang.IllegalArgumentException: timeout value is negative
        at java.lang.Thread.sleep(Thread.java:500)
        at com.microsoft.kiota.http.middleware.RetryHandler.retryRequest(RetryHandler.java:115)
        at com.microsoft.kiota.http.middleware.RetryHandler.intercept(RetryHandler.java:252)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.RedirectHandler.intercept(RedirectHandler.java:158)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.HeadersInspectionHandler.intercept(HeadersInspectionHandler.java:70)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.UrlReplaceHandler.intercept(UrlReplaceHandler.java:52)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.ParametersNameDecodingHandler.intercept(ParametersNameDecodingHandler.java:79)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.UserAgentHandler.intercept(UserAgentHandler.java:84)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.graph.core.requests.middleware.GraphTelemetryHandler.intercept(GraphTelemetryHandler.java:64)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at com.microsoft.kiota.http.middleware.UrlReplaceHandler.intercept(UrlReplaceHandler.java:72)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
        at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:201)
        at okhttp3.internal.connection.RealCall.execute(RealCall.kt:154)
        at com.microsoft.kiota.http.OkHttpRequestAdapter.getHttpResponseMessage(OkHttpRequestAdapter.java:749)
        at com.microsoft.kiota.http.OkHttpRequestAdapter.send(OkHttpRequestAdapter.java:302)
```</issue_description>

<agent_instructions>Fix it by setting a minimum random delay between 1 and 10 ms if the returned value is less than 0.
Make sure to add unit tests for that case.
Use conventional commit.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- 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 <[email protected]>
Copilot AI changed the title [WIP] Fix IllegalArgumentException in RetryHandler retryRequest fix: prevent negative timeout in RetryHandler causing IllegalArgumentException Nov 14, 2025
Copilot AI requested a review from baywet November 14, 2025 14:29
@github-project-automation github-project-automation bot moved this to In Progress 🚧 in Kiota Nov 14, 2025
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 <[email protected]>
Copilot AI requested a review from baywet November 14, 2025 14:48
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 <[email protected]>
Copilot AI requested a review from baywet November 14, 2025 14:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 14, 2025

@baywet baywet merged commit dd0be6c into main Nov 14, 2025
10 checks passed
@baywet baywet deleted the copilot/fix-retry-handler-timeout branch November 14, 2025 16:02
@github-project-automation github-project-automation bot moved this from In Progress 🚧 to Done ✔️ in Kiota Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

RetryHandler#retryRequest might fail with IllegalArgumentException: timeout value is negative

3 participants