Skip to content
This repository has been archived by the owner on Jul 9, 2024. It is now read-only.

Commit

Permalink
Resolve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
David Courtel committed Mar 25, 2024
1 parent 34f91da commit 376eb06
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 30 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -220,6 +221,8 @@ public async Task ExceedMaxRetryShouldReturn(HttpStatusCode statusCode)
Assert.IsType<AggregateException>(exception);
var aggregateException = exception as AggregateException;
Assert.StartsWith("Too many retries performed.", aggregateException.Message);
Assert.All(aggregateException.InnerExceptions, innerexception => Assert.IsType<ApiException>(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");
}
}
Expand Down
9 changes: 5 additions & 4 deletions Microsoft.Kiota.Http.HttpClientLibrary.sln
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Kiota.Http.HttpClientLibrary.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<PackageProjectUrl>https://aka.ms/kiota/docs</PackageProjectUrl>
<EmbedUntrackedSources>true</EmbedUntrackedSources>
<Deterministic>true</Deterministic>
<VersionPrefix>1.3.7</VersionPrefix>
<VersionPrefix>1.3.8</VersionPrefix>
<VersionSuffix></VersionSuffix>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<!-- Enable this line once we go live to prevent breaking changes -->
Expand Down
32 changes: 7 additions & 25 deletions src/Middleware/RetryHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -103,7 +105,7 @@ private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage respo

while(retryCount < retryOption.MaxRetry)
{
exceptions.Add(await GetInnerException(response, cancellationToken));
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);
Expand Down Expand Up @@ -148,7 +150,7 @@ private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage respo
}
}

exceptions.Add(await GetInnerException(response, cancellationToken));
exceptions.Add(await GetInnerExceptionAsync(response));

throw new AggregateException($"Too many retries performed. More than {retryCount} retries encountered while sending the request.", exceptions);
}
Expand Down Expand Up @@ -231,42 +233,22 @@ private static bool ShouldRetry(HttpStatusCode statusCode)
};
}

private static async Task<Exception> GetInnerExceptionAsync(HttpResponseMessage response, CancellationToken cancellationToken)

private static async Task<Exception> GetInnerExceptionAsync(HttpResponseMessage response)
{
var httpStatusCode = response.StatusCode;
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)
{
#if NET5_0_OR_GREATER
errorMessage = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
#else
errorMessage = await response.Content.ReadAsStringAsync().ConfigureAwait(false);

#endif
errorMessage = GetStringFromContent(responseContent);
}

return new ApiException($"HTTP request failed with status code: {httpStatusCode}.{errorMessage}")
return new ApiException($"HTTP request failed with status code: {response.StatusCode}.{errorMessage}")
{
ResponseStatusCode = (int)response.StatusCode,
ResponseHeaders = response.Headers.ToDictionary()
ResponseHeaders = response.Headers.ToDictionary(header => header.Key, header => header.Value),
};
}

private static string GetStringFromContent(byte[] content)
{
try
{
return System.Text.Encoding.UTF8.GetString(content);
}
catch(Exception)
{
return "The Graph retry handler was unable to cast the response into a UTF8 string.";
}
}
}
}

0 comments on commit 376eb06

Please sign in to comment.