diff --git a/CHANGELOG.md b/CHANGELOG.md index f69eced..9382cd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.3.8] - 2024-03-25] + +## Changed + +- When too many retries are attempted, the RetryHandler will now throw an `AggregateException` (instead of an `InvalidOperationException`). + The `InnerExceptions` property of the `AggregateException` will contain a list of `ApiException` with the HTTP status code and an error message if available. + ## [1.3.7] - 2024-02-26 ### Changed diff --git a/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/RetryHandlerTests.cs b/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/RetryHandlerTests.cs index 805fc68..426490b 100644 --- a/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/RetryHandlerTests.cs +++ b/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/RetryHandlerTests.cs @@ -6,6 +6,7 @@ using System.Net.Http; using System.Threading; using System.Threading.Tasks; +using Microsoft.Kiota.Abstractions; using Microsoft.Kiota.Http.HttpClientLibrary.Middleware; using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options; using Microsoft.Kiota.Http.HttpClientLibrary.Tests.Mocks; @@ -51,7 +52,6 @@ public void RetryHandlerConstructor() Assert.IsType(retry); } - [Fact] public void RetryHandlerHttpMessageHandlerConstructor() { @@ -89,7 +89,6 @@ public async Task OkStatusShouldPassThrough() Assert.NotNull(response.RequestMessage); Assert.Same(response.RequestMessage, httpRequestMessage); Assert.False(response.RequestMessage.Headers.Contains(RetryAttempt), "The request add header wrong."); - } [Theory] @@ -116,7 +115,6 @@ public async Task ShouldRetryWithAddRetryAttemptHeader(HttpStatusCode statusCode Assert.Equal(values.First(), 1.ToString()); } - [Theory] [InlineData(HttpStatusCode.GatewayTimeout)] // 504 [InlineData(HttpStatusCode.ServiceUnavailable)] // 503 @@ -140,7 +138,6 @@ public async Task ShouldRetryWithBuffedContent(HttpStatusCode statusCode) Assert.NotNull(response.RequestMessage.Content); Assert.NotNull(response.RequestMessage.Content.Headers.ContentLength); Assert.Equal("Hello World", await response.RequestMessage.Content.ReadAsStringAsync()); - } [Theory] @@ -196,8 +193,7 @@ public async Task ShouldNotRetryWithPutStreaming(HttpStatusCode statusCode) Assert.Equal(response.RequestMessage.Content.Headers.ContentLength, -1); } - - [Theory(Skip = "Test takes a while to run")] + [Theory] [InlineData(HttpStatusCode.GatewayTimeout)] // 504 [InlineData(HttpStatusCode.ServiceUnavailable)] // 503 [InlineData((HttpStatusCode)429)] // 429 @@ -208,16 +204,25 @@ public async Task ExceedMaxRetryShouldReturn(HttpStatusCode statusCode) var retryResponse = new HttpResponseMessage(statusCode); var response2 = new HttpResponseMessage(statusCode); this._testHttpMessageHandler.SetHttpResponse(retryResponse, response2); + var retryHandler = new RetryHandler + { + InnerHandler = _testHttpMessageHandler, + RetryOption = new RetryHandlerOption { Delay = 1 } + }; + var invoker = new HttpMessageInvoker(retryHandler); // Act try { - await _invoker.SendAsync(httpRequestMessage, new CancellationToken()); + await invoker.SendAsync(httpRequestMessage, new CancellationToken()); } catch(Exception exception) { // Assert - Assert.IsType(exception); - Assert.Equal("Too many retries performed", exception.Message); + Assert.IsType(exception); + var aggregateException = exception as AggregateException; + Assert.StartsWith("Too many retries performed.", aggregateException.Message); + Assert.All(aggregateException.InnerExceptions, innerexception => Assert.IsType(innerexception)); + Assert.All(aggregateException.InnerExceptions, innerexception => Assert.True((innerexception as ApiException).ResponseStatusCode == (int)statusCode)); Assert.False(httpRequestMessage.Headers.TryGetValues(RetryAttempt, out _), "Don't set Retry-Attempt Header"); } } @@ -246,17 +251,16 @@ public async Task ShouldDelayBasedOnRetryAfterHeaderWithHttpDate(HttpStatusCode // Arrange var retryResponse = new HttpResponseMessage(statusCode); var futureTime = DateTime.Now + TimeSpan.FromSeconds(3);// 3 seconds from now - var futureTimeString = futureTime.ToString(CultureInfo.InvariantCulture.DateTimeFormat.RFC1123Pattern); + var futureTimeString = futureTime.ToString(CultureInfo.InvariantCulture.DateTimeFormat.RFC1123Pattern, CultureInfo.InvariantCulture); Assert.Contains("GMT", futureTimeString); // http date always end in GMT according to the spec - retryResponse.Headers.TryAddWithoutValidation(RetryAfter, futureTimeString); + Assert.True(retryResponse.Headers.TryAddWithoutValidation(RetryAfter, futureTimeString)); // Act await DelayTestWithMessage(retryResponse, 1, "Init"); // Assert Assert.Equal("Init Work 1", Message); } - - [Theory(Skip = "Skipped as this takes 9 minutes to run for each scenario")] // Takes 9 minutes to run for each scenario + [Theory] [InlineData(HttpStatusCode.GatewayTimeout)] // 504 [InlineData(HttpStatusCode.ServiceUnavailable)] // 503 [InlineData((HttpStatusCode)429)] // 429 @@ -269,7 +273,7 @@ public async Task ShouldDelayBasedOnExponentialBackOff(HttpStatusCode statusCode for(int count = 0; count < 3; count++) { // Act - await DelayTestWithMessage(retryResponse, count, "Init"); + await DelayTestWithMessage(retryResponse, count, "Init", 1); // Assert Assert.Equal(Message, compareMessage + count); } @@ -351,7 +355,6 @@ public async Task ShouldRetryBasedOnRetryAfterHeaderWithHttpDate(HttpStatusCode Assert.NotSame(response.RequestMessage, httpRequestMessage); } - [Theory] [InlineData(1, HttpStatusCode.BadGateway, true)] [InlineData(2, HttpStatusCode.BadGateway, true)] @@ -411,14 +414,16 @@ public async Task ShouldRetryBasedOnCustomShouldRetryDelegate(int expectedMaxRet catch(Exception exception) { // Assert - Assert.IsType(exception); + Assert.IsType(exception); + var aggregateException = exception as AggregateException; Assert.True(isExceptionExpected); - Assert.Equal("Too many retries performed", exception.Message); + Assert.StartsWith("Too many retries performed.", aggregateException.Message); + Assert.Equal(1 + expectedMaxRetry, aggregateException.InnerExceptions.Count); + Assert.All(aggregateException.InnerExceptions, innerexception => Assert.Contains(expectedStatusCode.ToString(), innerexception.Message)); } // Assert mockHttpMessageHandler.Protected().Verify>("SendAsync", Times.Exactly(1 + expectedMaxRetry), ItExpr.IsAny(), It.IsAny()); - } private async Task DelayTestWithMessage(HttpResponseMessage response, int count, string message, int delay = RetryHandlerOption.MaxDelay) diff --git a/Microsoft.Kiota.Http.HttpClientLibrary.sln b/Microsoft.Kiota.Http.HttpClientLibrary.sln index e6837d4..0251aa9 100644 --- a/Microsoft.Kiota.Http.HttpClientLibrary.sln +++ b/Microsoft.Kiota.Http.HttpClientLibrary.sln @@ -1,18 +1,19 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 16 -VisualStudioVersion = 16.0.30114.105 +# Visual Studio Version 17 +VisualStudioVersion = 17.9.34714.143 MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Kiota.Http.HttpClientLibrary", "src\Microsoft.Kiota.Http.HttpClientLibrary.csproj", "{769E84B2-FD14-4173-B83B-6792DFB0BA14}" EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{178B3904-FBB4-4E5A-A569-B5082BABC124}" ProjectSection(SolutionItems) = preProject .editorconfig = .editorconfig + CHANGELOG.md = CHANGELOG.md EndProjectSection EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Kiota.Http.HttpClientLibrary.Tests", "Microsoft.Kiota.Http.HttpClientLibrary.Tests\Microsoft.Kiota.Http.HttpClientLibrary.Tests.csproj", "{CCD20728-D593-48F8-8627-6E7A57B31A43}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Kiota.Http.HttpClientLibrary.Tests", "Microsoft.Kiota.Http.HttpClientLibrary.Tests\Microsoft.Kiota.Http.HttpClientLibrary.Tests.csproj", "{CCD20728-D593-48F8-8627-6E7A57B31A43}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "KiotaGenerated", "Kiota.Generated\KiotaGenerated.csproj", "{6667C82F-EFF1-4D7C-828D-F981629C8256}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "KiotaGenerated", "Kiota.Generated\KiotaGenerated.csproj", "{6667C82F-EFF1-4D7C-828D-F981629C8256}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj b/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj index 5bfda1d..e3efef6 100644 --- a/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj +++ b/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj @@ -14,7 +14,7 @@ https://aka.ms/kiota/docs true true - 1.3.7 + 1.3.8 true diff --git a/src/Middleware/RetryHandler.cs b/src/Middleware/RetryHandler.cs index 56df4ca..b98c652 100644 --- a/src/Middleware/RetryHandler.cs +++ b/src/Middleware/RetryHandler.cs @@ -5,12 +5,14 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks; +using Microsoft.Kiota.Abstractions; using Microsoft.Kiota.Http.HttpClientLibrary.Extensions; using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options; @@ -46,6 +48,7 @@ public RetryHandler(RetryHandlerOption? retryOption = null) /// /// The HTTP requestneeds to be sent. /// The for the request. + /// Thrown when too many retries are performed. /// protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { @@ -69,7 +72,6 @@ protected override async Task SendAsync(HttpRequestMessage try { - 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 @@ -93,27 +95,20 @@ protected override async Task SendAsync(HttpRequestMessage /// The for the retry. /// The for the retry. /// The for the retry. + /// Thrown when too many retries are performed." /// private async Task SendRetryAsync(HttpResponseMessage response, RetryHandlerOption retryOption, CancellationToken cancellationToken, ActivitySource? activitySource) { int retryCount = 0; TimeSpan cumulativeDelay = TimeSpan.Zero; + List exceptions = new(); while(retryCount < retryOption.MaxRetry) { + exceptions.Add(await GetInnerExceptionAsync(response)); using var retryActivity = activitySource?.StartActivity($"{nameof(RetryHandler)}_{nameof(SendAsync)} - attempt {retryCount}"); retryActivity?.SetTag("http.retry_count", retryCount); retryActivity?.SetTag("http.status_code", response.StatusCode); - // Drain response content to free connections. Need to perform this - // before retry attempt and before the TooManyRetries ServiceException. - if(response.Content != null) - { -#if NET5_0_OR_GREATER - await response.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false); -#else - await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false); -#endif - } // Call Delay method to get delay time from response's Retry-After header or by exponential backoff Task delay = RetryHandler.Delay(response, retryCount, retryOption.Delay, out double delayInSeconds, cancellationToken); @@ -155,20 +150,9 @@ private async Task SendRetryAsync(HttpResponseMessage respo } } - // Drain response content to free connections. Need to perform this - // before retry attempt and before the TooManyRetries ServiceException. - if(response.Content != null) - { -#if NET5_0_OR_GREATER - await response.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false); -#else - await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false); -#endif - } + exceptions.Add(await GetInnerExceptionAsync(response)); - throw new InvalidOperationException( - "Too many retries performed", - new Exception($"More than {retryCount} retries encountered while sending the request.")); + throw new AggregateException($"Too many retries performed. More than {retryCount} retries encountered while sending the request.", exceptions); } /// @@ -248,5 +232,23 @@ private static bool ShouldRetry(HttpStatusCode statusCode) _ => false }; } + + private static async Task GetInnerExceptionAsync(HttpResponseMessage response) + { + string? errorMessage = null; + + // Drain response content to free connections. Need to perform this + // before retry attempt and before the TooManyRetries ServiceException. + if(response.Content != null) + { + errorMessage = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + } + + return new ApiException($"HTTP request failed with status code: {response.StatusCode}.{errorMessage}") + { + ResponseStatusCode = (int)response.StatusCode, + ResponseHeaders = response.Headers.ToDictionary(header => header.Key, header => header.Value), + }; + } } }