From 306b2d06d62f625284d5a0fe637699775d26e5b8 Mon Sep 17 00:00:00 2001 From: Andrew Bennet Date: Mon, 2 Feb 2026 18:07:30 +0000 Subject: [PATCH 1/3] Move the default should-retry logic into the default RetryOption so that it can be overridden --- .../httpClient/Middleware/RetryHandler.cs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/http/httpClient/Middleware/RetryHandler.cs b/src/http/httpClient/Middleware/RetryHandler.cs index 97fb4140..1d8a3770 100644 --- a/src/http/httpClient/Middleware/RetryHandler.cs +++ b/src/http/httpClient/Middleware/RetryHandler.cs @@ -38,7 +38,17 @@ internal RetryHandlerOption RetryOption /// An OPTIONAL to configure public RetryHandler(RetryHandlerOption? retryOption = null) { - RetryOption = retryOption ?? new RetryHandlerOption(); + RetryOption = retryOption ?? new RetryHandlerOption() + { + ShouldRetry = (_, _, response) => response.StatusCode switch + { + // By default, retry on 503, 504, and 429 status codes + HttpStatusCode.ServiceUnavailable => true, + HttpStatusCode.GatewayTimeout => true, + (HttpStatusCode)429 => true, + _ => false + } + }; } /// @@ -73,7 +83,7 @@ protected override async Task SendAsync(HttpRequestMessage var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); // Check whether retries are permitted and that the MaxRetry value is a non - negative, non - zero value - if(request.IsBuffered() && retryOption.MaxRetry > 0 && (ShouldRetry(response.StatusCode) || retryOption.ShouldRetry(retryOption.Delay, 0, response))) + if(request.IsBuffered() && retryOption.MaxRetry > 0 && retryOption.ShouldRetry(retryOption.Delay, 0, response)) { response = await SendRetryAsync(response, retryOption, cancellationToken, activitySource).ConfigureAwait(false); } @@ -143,7 +153,7 @@ private async Task SendRetryAsync(HttpResponseMessage respo // Call base.SendAsync to send the request response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - if(!(request.IsBuffered() && (ShouldRetry(response.StatusCode) || retryOption.ShouldRetry(retryOption.Delay, retryCount, response)))) + if(!(request.IsBuffered() && retryOption.ShouldRetry(retryOption.Delay, retryCount, response))) { return response; } @@ -217,22 +227,6 @@ private static double CalculateExponentialDelay(int retryCount, int delay) return Math.Pow(2, retryCount) * delay; } - /// - /// Check the HTTP status to determine whether it should be retried or not. - /// - /// The returned. - /// - private static bool ShouldRetry(HttpStatusCode statusCode) - { - return statusCode switch - { - HttpStatusCode.ServiceUnavailable => true, - HttpStatusCode.GatewayTimeout => true, - (HttpStatusCode)429 => true, - _ => false - }; - } - private static async Task GetInnerExceptionAsync(HttpResponseMessage response) { string? errorMessage = null; From 4b2696180b44c52723d1930cb356798e4c7be2c4 Mon Sep 17 00:00:00 2001 From: Andrew Bennet Date: Thu, 5 Feb 2026 10:29:47 +0000 Subject: [PATCH 2/3] Relocate default ShouldRetry implementation to be in the RetryOptions rather than the RetryHandler --- .../Middleware/Options/RetryHandlerOption.cs | 13 +++++++++++-- src/http/httpClient/Middleware/RetryHandler.cs | 12 +----------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/http/httpClient/Middleware/Options/RetryHandlerOption.cs b/src/http/httpClient/Middleware/Options/RetryHandlerOption.cs index 7f0a7cab..b4afff2b 100644 --- a/src/http/httpClient/Middleware/Options/RetryHandlerOption.cs +++ b/src/http/httpClient/Middleware/Options/RetryHandlerOption.cs @@ -3,6 +3,7 @@ // ------------------------------------------------------------------------------ using System; +using System.Net; using System.Net.Http; using Microsoft.Kiota.Abstractions; @@ -66,8 +67,16 @@ public int MaxRetry /// /// A delegate that's called to determine whether a request should be retried or not. - /// The delegate method should accept a delay time in seconds of, number of retry attempts and as it's parameters and return a . This defaults to false + /// The delegate method should accept a delay time in seconds of, number of retry attempts and as its parameters and return a . + /// This defaults to a function that returns true for 503, 504, and 429 status codes and false otherwise. /// - public Func ShouldRetry { get; set; } = (_, _, _) => false; + public Func ShouldRetry { get; set; } = (_, _, response) => response.StatusCode switch + { + // By default, retry on 503, 504, and 429 status codes + HttpStatusCode.ServiceUnavailable => true, + HttpStatusCode.GatewayTimeout => true, + (HttpStatusCode)429 => true, + _ => false + }; } } diff --git a/src/http/httpClient/Middleware/RetryHandler.cs b/src/http/httpClient/Middleware/RetryHandler.cs index 1d8a3770..0ec04e69 100644 --- a/src/http/httpClient/Middleware/RetryHandler.cs +++ b/src/http/httpClient/Middleware/RetryHandler.cs @@ -38,17 +38,7 @@ internal RetryHandlerOption RetryOption /// An OPTIONAL to configure public RetryHandler(RetryHandlerOption? retryOption = null) { - RetryOption = retryOption ?? new RetryHandlerOption() - { - ShouldRetry = (_, _, response) => response.StatusCode switch - { - // By default, retry on 503, 504, and 429 status codes - HttpStatusCode.ServiceUnavailable => true, - HttpStatusCode.GatewayTimeout => true, - (HttpStatusCode)429 => true, - _ => false - } - }; + RetryOption = retryOption ?? new RetryHandlerOption(); } /// From 3202a40d4f800ba35889317017c8c87ae4ad8888 Mon Sep 17 00:00:00 2001 From: Andrew Bennet Date: Thu, 5 Feb 2026 11:33:20 +0000 Subject: [PATCH 3/3] Add unit test that setting ShouldRetry to a false-returning delegate turns of the default retries of 429, 503 and 504 responses --- .../Middleware/RetryHandlerTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/http/httpClient/Middleware/RetryHandlerTests.cs b/tests/http/httpClient/Middleware/RetryHandlerTests.cs index fd233520..6f61f996 100644 --- a/tests/http/httpClient/Middleware/RetryHandlerTests.cs +++ b/tests/http/httpClient/Middleware/RetryHandlerTests.cs @@ -424,6 +424,35 @@ public async Task ShouldRetryBasedOnCustomShouldRetryDelegate(int expectedMaxRet // Assert mockHttpMessageHandler.Protected().Verify>("SendAsync", Times.Exactly(1 + expectedMaxRetry), ItExpr.IsAny(), It.IsAny()); + } + + [Theory] + [InlineData(HttpStatusCode.GatewayTimeout)] // 504 + [InlineData(HttpStatusCode.ServiceUnavailable)] // 503 + [InlineData((HttpStatusCode)429)] // 429 + public async Task ShouldNotRetryWithCustomShouldRetryDelegate(HttpStatusCode statusCode) + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, "http://example.org/foo") + { + Content = new StringContent("Test Content") + }; + httpRequestMessage.Content.Headers.ContentLength = -1; + + var retryResponse = new HttpResponseMessage(statusCode); + var response2 = new HttpResponseMessage(HttpStatusCode.OK); + this._testHttpMessageHandler.SetHttpResponse(retryResponse, response2); + _retryHandler.RetryOption.ShouldRetry = (_, _, _) => false; + // Act + var response = await _invoker.SendAsync(httpRequestMessage, new CancellationToken()); + // Assert + Assert.NotEqual(response, response2); + Assert.Same(response, retryResponse); + Assert.Same(response.RequestMessage, httpRequestMessage); + Assert.NotNull(response.RequestMessage); + Assert.NotNull(response.RequestMessage.Content); + Assert.NotNull(response.RequestMessage.Content.Headers.ContentLength); + Assert.Equal(response.RequestMessage.Content.Headers.ContentLength, -1); } private async Task DelayTestWithMessage(HttpResponseMessage response, int count, string message, int delay = RetryHandlerOption.MaxDelay)