From 2739017e9731a7722a63b1814b749e5562a6b848 Mon Sep 17 00:00:00 2001 From: Iliar Turdushev Date: Mon, 9 Sep 2024 12:45:02 +0200 Subject: [PATCH] Disable HttpClient's timeout for Standard Resilience and Hedging (#5363) * Fixes #4924 and #4770 Disables HttpClient Timeout for standard resilience and hending handlers * Fixes #4924 Adds a note mentioning requirements on the Grpc.Net.ClientFactory version to avoid issues with the M.E.Http.Resilience package * Fixes #4924 Added a target that notifies users that they use a version of the Grpc.Net.ClientFactory package that might cause the #4924 issue * Fixes #4924 Revert changes to the Directory.Build.targets * Fixes #4924 Adds a target that checks whether M.E.Http.Resilience package is used together with Grpc.Net.ClientFactory 2.64.0 or later. If not the target warns a user. * Fixes #4924 Adds a Known issues section to the doc describing the issue with Grpc.Net.ClientFactory * Fixes #4924 * Moves the contents of the .props file into the .targets file * For net462 we now import the contents of the .targets file instead of setting it as a CDATA value in the .csproj file * Fixes #4924 Replaces the name of the project file with the MSBuildProjectName variable * Fixes #4924 * Add conditions to pack buildTransitive .targets for net462 only when net462 is included as a target framework * Changed the documentation link to the learn.microsoft.com site * Fixes #4924 * Applies editorial changes to the Known issues section * Fixes #4924 * Changes the level of the compatibility log messages from Error to Warning * Updates the logic of copying buildTransitive files * Removed extra spaces * Applied suggestions to update the documentation * Removed locale from the URL --- eng/MSBuild/Packaging.targets | 8 +-- eng/packages/TestOnly.props | 2 +- ...enceHttpClientBuilderExtensions.Hedging.cs | 4 ++ ...icrosoft.Extensions.Http.Resilience.csproj | 35 ++++++++++++ .../README.md | 30 +++++++++++ ...entBuilderExtensions.StandardResilience.cs | 4 ++ ...crosoft.Extensions.Http.Resilience.targets | 54 +++++++++++++++++++ .../Hedging/StandardHedgingTests.cs | 8 +++ ...tpClientBuilderExtensionsTests.Standard.cs | 11 ++++ 9 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets diff --git a/eng/MSBuild/Packaging.targets b/eng/MSBuild/Packaging.targets index e233142f1fa..14a5a04d492 100644 --- a/eng/MSBuild/Packaging.targets +++ b/eng/MSBuild/Packaging.targets @@ -11,15 +11,17 @@ false $(BeforePack);_VerifyMinimumSupportedTfmForPackagingIsUsed;_AddNETStandardCompatErrorFileForPackaging + true + Condition="'$(IsPackNet462)' == 'true'" /> diff --git a/eng/packages/TestOnly.props b/eng/packages/TestOnly.props index 2f35392510e..df39b892596 100644 --- a/eng/packages/TestOnly.props +++ b/eng/packages/TestOnly.props @@ -5,7 +5,7 @@ - + diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs index e6039bd65c4..3f7b82ba0de 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs @@ -3,6 +3,7 @@ using System; using System.Net.Http; +using System.Threading; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Http.Resilience; @@ -139,6 +140,9 @@ public static IStandardHedgingHandlerBuilder AddStandardHedgingHandler(this IHtt }) .SelectPipelineByAuthority(); + // Disable the HttpClient timeout to allow the timeout strategies to control the timeout. + _ = builder.ConfigureHttpClient(client => client.Timeout = Timeout.InfiniteTimeSpan); + return new StandardHedgingHandlerBuilder(builder.Name, builder.Services, routingBuilder); } diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Microsoft.Extensions.Http.Resilience.csproj b/src/Libraries/Microsoft.Extensions.Http.Resilience/Microsoft.Extensions.Http.Resilience.csproj index 7bf27efddb3..f0499dada26 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Microsoft.Extensions.Http.Resilience.csproj +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Microsoft.Extensions.Http.Resilience.csproj @@ -38,4 +38,39 @@ + + + + + + + + + + + + + <_AdditionalNETStandardCompatErrorFileContents> + +]]> + + diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/README.md b/src/Libraries/Microsoft.Extensions.Http.Resilience/README.md index a157dcfa4bc..d4ad24b5e00 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/README.md +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/README.md @@ -73,6 +73,36 @@ clientBuilder.AddResilienceHandler("myHandler", b => }); ``` +## Known issues + +The following sections detail various known issues. + +### Compatibility with the `Grpc.Net.ClientFactory` package + +If you're using `Grpc.Net.ClientFactory` version `2.63.0` or earlier, then enabling the standard resilience or hedging handlers for a gRPC client could cause a runtime exception. Specifically, consider the following code sample: + +```csharp +services + .AddGrpcClient() + .AddStandardResilienceHandler(); +``` + +The preceding code results in the following exception: + +```Output +System.InvalidOperationException: The ConfigureHttpClient method is not supported when creating gRPC clients. Unable to create client with name 'GreeterClient'. +``` + +To resolve this issue, we recommend upgrading to `Grpc.Net.ClientFactory` version `2.64.0` or later. + +There's a build time check that verifies if you're using `Grpc.Net.ClientFactory` version `2.63.0` or earlier, and if you are the check produces a compilation warning. You can suppress the warning by setting the following property in your project file: + +```xml + + true + +``` + ## Feedback & Contributing We welcome feedback and contributions in [our GitHub repo](https://github.com/dotnet/extensions). diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.StandardResilience.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.StandardResilience.cs index f27c2e76eac..a4315eaa006 100644 --- a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.StandardResilience.cs +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHttpClientBuilderExtensions.StandardResilience.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Threading; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Http.Resilience; @@ -86,6 +87,9 @@ public static IHttpStandardResiliencePipelineBuilder AddStandardResilienceHandle .AddTimeout(options.AttemptTimeout); }); + // Disable the HttpClient timeout to allow the timeout strategies to control the timeout. + _ = builder.ConfigureHttpClient(client => client.Timeout = Timeout.InfiniteTimeSpan); + return new HttpStandardResiliencePipelineBuilder(optionsName, builder.Services); } diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets b/src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets new file mode 100644 index 00000000000..268f0720433 --- /dev/null +++ b/src/Libraries/Microsoft.Extensions.Http.Resilience/buildTransitive/Microsoft.Extensions.Http.Resilience.targets @@ -0,0 +1,54 @@ + + + <_GrpcNetClientFactory>Grpc.Net.ClientFactory + <_CompatibleGrpcNetClientFactoryVersion>2.64.0 + <_GrpcNetClientFactoryVersionIsIncorrect>Grpc.Net.ClientFactory 2.63.0 or earlier could cause issues when used together with Microsoft.Extensions.Http.Resilience. For more details, see https://learn.microsoft.com/dotnet/core/resilience/http-resilience#known-issues. Consider using Grpc.Net.ClientFactory $(_CompatibleGrpcNetClientFactoryVersion) or later. To suppress the warning set SuppressCheckGrpcNetClientFactoryVersion=true. + + + + + + + <_GrpcNetClientFactoryPackageReference Include="@(PackageReference)" Condition=" '%(PackageReference.Identity)' == '$(_GrpcNetClientFactory)' " /> + + + <_GrpcNetClientFactoryPackageVersion Include="@(PackageVersion)" Condition=" '%(PackageVersion.Identity)' == '$(_GrpcNetClientFactory)' " /> + + + <_GrpcNetClientFactoryTransitiveDependency Include="@(ReferencePath)" Condition=" '%(ReferencePath.NuGetPackageId)' == '$(_GrpcNetClientFactory)' " /> + + + + + + + + + + + + + + + diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Hedging/StandardHedgingTests.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Hedging/StandardHedgingTests.cs index 237ccfa6e73..7db71a08873 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Hedging/StandardHedgingTests.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Hedging/StandardHedgingTests.cs @@ -265,6 +265,14 @@ public async Task DynamicReloads_Ok(bool asynchronous = true) AssertNoResponse(); } + [Fact] + public void AddStandardResilienceHandler_EnsureHttpClientTimeoutDisabled() + { + var client = CreateClientWithHandler(); + + client.Timeout.Should().Be(Timeout.InfiniteTimeSpan); + } + [Theory] #if NET6_0_OR_GREATER [CombinatorialData] diff --git a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Standard.cs b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Standard.cs index 79ce7aa654d..faecd6e317d 100644 --- a/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Standard.cs +++ b/test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/HttpClientBuilderExtensionsTests.Standard.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using FluentAssertions; using Microsoft.Extensions.Configuration; @@ -257,6 +258,16 @@ public async Task DynamicReloads_Ok(bool asynchronous = true) requests.Should().HaveCount(11); } + [Fact] + public void AddStandardResilienceHandler_EnsureHttpClientTimeoutDisabled() + { + var builder = new ServiceCollection().AddLogging().AddMetrics().AddHttpClient("test").AddStandardResilienceHandler(); + + using var client = builder.Services.BuildServiceProvider().GetRequiredService().CreateClient("test"); + + client.Timeout.Should().Be(Timeout.InfiniteTimeSpan); + } + private static void AddStandardResilienceHandler( MethodArgs mode, IHttpClientBuilder builder,