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.

Sign up for GitHub

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

Merge changes from internal branch #5158

Merged
merged 9 commits into from
May 14, 2024
Merged

Merge changes from internal branch #5158

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
26047a0
Detect connection timeouts in Hedging
xakep139 Apr 30, 2024
2f5ba8c
fix the test
xakep139 May 2, 2024
21f3d17
CR
xakep139 May 6, 2024
72fdb38
CR, 2nd round
xakep139 May 7, 2024
6d3eb4c
Use connection timeout check in common strategies
xakep139 May 7, 2024
80d518b
add more tests
xakep139 May 9, 2024
de1f323
[release/8.5] Detect connection timeouts in Hedging (#5149)
joperezr May 9, 2024
659966d
Merged PR 39515: Getting ready for 8.5 Release
joperezr May 13, 2024
231e807
Merge changes from internal branch
joperezr May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
CR
  • Loading branch information
@xakep139
xakep139 authored and github-actions committed May 9, 2024
commit 21f3d17b4f917766dfc73bbd0fe3ed966e4a8cb1
10 changes: 9 additions & 1 deletion ...ies/Microsoft.Extensions.Http.Resilience/Hedging/HttpClientHedgingResiliencePredicates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Net.Http;
using Microsoft.Shared.DiagnosticIds;
using Microsoft.Shared.Diagnostics;
using Polly;
using Polly.CircuitBreaker;
Expand All @@ -27,7 +29,13 @@ public static bool IsTransient(Outcome<HttpResponseMessage> outcome)
_ => false,
};

internal static bool IsTransient(HedgingPredicateArguments<HttpResponseMessage> args)
/// <summary>
/// Determines whether an <see cref="HttpResponseMessage"/> should be treated by hedging as a transient failure.
/// </summary>
/// <param name="args">A <see cref="HedgingPredicateArguments{T}"/>.</param>
/// <returns><see langword="true"/> if outcome is transient, <see langword="false"/> if not.</returns>
[Experimental(diagnosticId: DiagnosticIds.Experiments.Resilience, UrlFormat = DiagnosticIds.UrlFormat)]
public static bool IsTransient(HedgingPredicateArguments<HttpResponseMessage> args)
=> IsConnectionTimeout(args)
|| IsTransient(args.Outcome);

Expand Down
19 changes: 6 additions & 13 deletions test/Libraries/Microsoft.Extensions.Http.Resilience.Tests/Hedging/StandardHedgingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ public void EnsureValidated_BasicValidation()
{
Builder.Configure(options => options.Hedging.MaxHedgedAttempts = -1);

Assert.Throws<OptionsValidationException>(() => CreateClientWithHandler());
Assert.Throws<OptionsValidationException>(CreateClientWithHandler);
}

[Fact]
public void EnsureValidated_AdvancedValidation()
{
Builder.Configure(options => options.TotalRequestTimeout.Timeout = TimeSpan.FromSeconds(1));

Assert.Throws<OptionsValidationException>(() => CreateClientWithHandler());
Assert.Throws<OptionsValidationException>(CreateClientWithHandler);
}

[Fact]
Expand Down Expand Up @@ -276,7 +276,7 @@ public async Task SendAsync_FailedConnect_ShouldReturnResponseFromHedging()
const string FailingEndpoint = "www.failing-host.com";

var services = new ServiceCollection();
var clientBuilder = services
_ = services
.AddHttpClient(ClientId)
.ConfigurePrimaryHttpMessageHandler(() => new MockHttpMessageHandler(FailingEndpoint))
.AddStandardHedgingHandler(routing =>
Expand All @@ -302,7 +302,7 @@ public async Task SendAsync_FailedConnect_ShouldReturnResponseFromHedging()
opt.Endpoint.Timeout.Timeout = TimeSpan.FromSeconds(200);
});

using var provider = services.BuildServiceProvider();
await using var provider = services.BuildServiceProvider();
var clientFactory = provider.GetRequiredService<IHttpClientFactory>();
using var client = clientFactory.CreateClient(ClientId);

Expand All @@ -316,18 +316,11 @@ public async Task SendAsync_FailedConnect_ShouldReturnResponseFromHedging()

protected override void ConfigureHedgingOptions(Action<HttpHedgingStrategyOptions> configure) => Builder.Configure(options => configure(options.Hedging));

private class MockHttpMessageHandler : HttpMessageHandler
private class MockHttpMessageHandler(string failingEndpoint) : HttpMessageHandler
{
private readonly string _failingEndpoint;

public MockHttpMessageHandler(string failingEndpoint)
{
_failingEndpoint = failingEndpoint;
}

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
if (request.RequestUri?.Host == _failingEndpoint)
if (request.RequestUri?.Host == failingEndpoint)
{
await Task.Delay(100, cancellationToken);
throw new OperationCanceledExceptionMock(new TimeoutException());
Expand Down