Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable HttpClient's timeout for Standard Resilience and Hedging #5363

Merged
8 changes: 5 additions & 3 deletions eng/MSBuild/Packaging.targets
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
<PropertyGroup>
<IsPackable Condition="'$(Stage)' == ''">false</IsPackable>
<BeforePack>$(BeforePack);_VerifyMinimumSupportedTfmForPackagingIsUsed;_AddNETStandardCompatErrorFileForPackaging</BeforePack>
<IsPackNet462 Condition="'$(MinimumSupportedTfmForPackaging)' != '' and
'$(ConditionalNet462)' != '' and
($(TargetFrameworks.Contains('$(MinimumSupportedTfmForPackaging)')) and $(TargetFrameworks.Contains('$(ConditionalNet462)')))"
>true</IsPackNet462>
</PropertyGroup>

<!-- Include a compat warning for users trying to use our packages on unsupported TFMs -->
<ItemGroup>
<NETStandardCompatError Include="$(ConditionalNet462)"
Supported="$(MinimumSupportedTfmForPackaging)"
Condition="'$(MinimumSupportedTfmForPackaging)' != '' and
'$(ConditionalNet462)' != '' and
($(TargetFrameworks.Contains('$(MinimumSupportedTfmForPackaging)')) and $(TargetFrameworks.Contains('$(ConditionalNet462)')))" />
Condition="'$(IsPackNet462)' == 'true'" />
</ItemGroup>

<PropertyGroup Condition=" '$(IsPackable)' == 'true' and '$(IsShipping)' == 'true' ">
Expand Down
2 changes: 1 addition & 1 deletion eng/packages/TestOnly.props
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<PackageVersion Include="autofixture" Version="4.17.0" />
<PackageVersion Include="BenchmarkDotNet" Version="0.13.5" />
<PackageVersion Include="FluentAssertions" Version="6.11.0" />
<PackageVersion Include="Grpc.AspNetCore" Version="2.57.0" />
<PackageVersion Include="Grpc.AspNetCore" Version="2.65.0" />
<PackageVersion Include="Microsoft.Diagnostics.Tracing.TraceEvent" Version="3.1.3" />
<PackageVersion Include="Moq.AutoMock" Version="3.1.0" />
<PackageVersion Include="Moq" Version="4.18.4" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,39 @@
<InternalsVisibleToTest Include="$(AssemblyName).Tests" />
<InternalsVisibleToDynamicProxyGenAssembly2 Include="*" />
</ItemGroup>

<ItemGroup>
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved
<None Include="buildTransitive\**\*"
CopyToOutputDirectory="PreserveNewest"
Pack="true"
PackagePath="buildTransitive\$(MinimumSupportedTfmForPackaging)\"
Condition="'$(MinimumSupportedTfmForPackaging)' != '' and $(TargetFrameworks.Contains('$(MinimumSupportedTfmForPackaging)'))" />

<!-- For net462 we copy all buildTransitive files except the .targets file. We have to rename the .targets file
when copying it because our build process automatically adds a .targets file with the same name as the project
file. Then we'll import the renamed .targets file in the automatically added .targets file. For that we need
to set the _AdditionalNETStandardCompatErrorFileContents variable. -->
<None Include="buildTransitive\**\*"
Exclude="buildTransitive\$(MSBuildProjectName).targets"
CopyToOutputDirectory="PreserveNewest"
Pack="true"
PackagePath="buildTransitive\net462\"
Condition="'$(IsPackNet462)' == 'true'" />

<None Include="buildTransitive\$(MSBuildProjectName).targets"
CopyToOutputDirectory="PreserveNewest"
Pack="true"
PackagePath="buildTransitive\net462\$(MSBuildProjectName).net462.targets"
Condition="'$(IsPackNet462)' == 'true'" />
</ItemGroup>
RussKie marked this conversation as resolved.
Show resolved Hide resolved

<!-- For net462 we automatically add the .targets file, and to include the code that checks the version of the
Grpc.Net.ClientFactory package we need to set the _AdditionalNETStandardCompatErrorFileContents variable. -->
<PropertyGroup>
<_AdditionalNETStandardCompatErrorFileContents>
<![CDATA[
<Import Project="%24(MSBuildThisFileDirectory)\$(MSBuildProjectName).net462.targets" />
]]>
</_AdditionalNETStandardCompatErrorFileContents>
</PropertyGroup>
</Project>
30 changes: 30 additions & 0 deletions src/Libraries/Microsoft.Extensions.Http.Resilience/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,36 @@ clientBuilder.AddResilienceHandler("myHandler", b =>
});
```

## Known issues
iliar-turdushev marked this conversation as resolved.
Show resolved Hide resolved

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<Greeter.GreeterClient>()
.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
<PropertyGroup>
<SuppressCheckGrpcNetClientFactoryVersion>true</SuppressCheckGrpcNetClientFactoryVersion>
</PropertyGroup>
```

## Feedback & Contributing

We welcome feedback and contributions in [our GitHub repo](https://github.com/dotnet/extensions).
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<Project>
<PropertyGroup>
<_GrpcNetClientFactory>Grpc.Net.ClientFactory</_GrpcNetClientFactory>
<_CompatibleGrpcNetClientFactoryVersion>2.64.0</_CompatibleGrpcNetClientFactoryVersion>
<_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.</_GrpcNetClientFactoryVersionIsIncorrect>
</PropertyGroup>

<!--
Check whether the project is referencing Grpc.Net.ClientFactory 2.64.0 or later.
If the vesion is earlier warn the user to update.
See https://learn.microsoft.com/en-us/dotnet/core/resilience/http-resilience#known-issues for more details.
-->
<Target Name="_CheckGrpcNetClientFactoryVersion"
BeforeTargets="ResolveReferences"
Condition=" '$(SuppressCheckGrpcNetClientFactoryVersion)' != 'true' ">
<ItemGroup>
<!-- Find the package in the .csproj file. -->
<_GrpcNetClientFactoryPackageReference Include="@(PackageReference)" Condition=" '%(PackageReference.Identity)' == '$(_GrpcNetClientFactory)' " />

<!-- Find the version of the package in the Central Package Source. The solution uses the Central Package Management. -->
<_GrpcNetClientFactoryPackageVersion Include="@(PackageVersion)" Condition=" '%(PackageVersion.Identity)' == '$(_GrpcNetClientFactory)' " />

<!-- The package is added to the project as a transitive dependency. -->
<_GrpcNetClientFactoryTransitiveDependency Include="@(ReferencePath)" Condition=" '%(ReferencePath.NuGetPackageId)' == '$(_GrpcNetClientFactory)' " />
</ItemGroup>

<!-- The version of the package is included in the .csproj file. -->
<Warning Condition=" @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
AND '%(_GrpcNetClientFactoryPackageReference.Version)' != ''
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference.Version)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />

<!-- The solution uses the Central Package Management and the version of the package is overridden in the .csproj file using the VersionOverride property. -->
<Warning Condition=" '$(ManagePackageVersionsCentrally)' == 'true'
AND @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
AND '%(_GrpcNetClientFactoryPackageReference.VersionOverride)' != ''
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageReference.VersionOverride)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />

<!-- The solution uses the Central Package Management and the version of the package is included in the Central Package Source. -->
<Warning Condition=" '$(ManagePackageVersionsCentrally)' == 'true'
AND @(_GrpcNetClientFactoryPackageReference->Count()) &gt; 0
AND '%(_GrpcNetClientFactoryPackageVersion.Version)' != ''
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryPackageVersion.Version)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />

<!-- This condition handles a case when the package is added to the project as a transitive dependency. -->
<Warning Condition=" @(_GrpcNetClientFactoryPackageReference->Count()) == 0
AND @(_GrpcNetClientFactoryTransitiveDependency->Count()) &gt; 0
AND '%(_GrpcNetClientFactoryTransitiveDependency.NuGetPackageVersion)' != ''
AND $([MSBuild]::VersionLessThan('%(_GrpcNetClientFactoryTransitiveDependency.NuGetPackageVersion)', '$(_CompatibleGrpcNetClientFactoryVersion)')) "
Text="$(_GrpcNetClientFactoryVersionIsIncorrect)" />
</Target>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<IHttpClientFactory>().CreateClient("test");

client.Timeout.Should().Be(Timeout.InfiniteTimeSpan);
}

private static void AddStandardResilienceHandler(
MethodArgs mode,
IHttpClientBuilder builder,
Expand Down