tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.15k stars 385 forks source link

Retry policies #346

Open Pinox opened 6 years ago

Pinox commented 6 years ago

Hi Guys,

Saw this on hanselman blog. freaking awesome lib.

I don't see anything in the docs like a retrycount where the httpclient can retry the request on timeout .

==> "http://localhost:7071/api/request" .RetryCount(3) //retry 3 times

or .RetryCount(3 , 300) //retry 3 times interval 300 milliseconds

tmenier commented 6 years ago

Thanks. It's a good idea and one I've thought about before. Have you checked out Polly? It's the ultimate lib for implementing retries and I recommend using it with Flurl. So I'm not sure if I want to move into that territory directly, but for a simple scenario like this I might consider it. I'll give it some thought.

Pinox commented 6 years ago

Hi @tmenier, thanks, I have seen it before but at the time when considering what I need at 272Kb file size for polly I thought it's not worth it as I will also use it in Xamarin Forms.

On the topic of third party integration , I currently use https://github.com/alexrainman/ModernHttpClient to use native methods in Android and iOS.

Is integration with such library possible with Flurl ? thanks in advance.

tmenier commented 6 years ago

https://github.com/tmenier/Flurl/issues/24 ;)

tmenier commented 5 years ago

I'm actively gathering feedback to help prioritize issues for 3.0. If this one is important to you, please vote for it here!

SaahilClaypool commented 2 years ago

I had some trouble adding policies other than using a HTTP client factory, so I wrote a nuget package call PollyFlurl to add policies in a fluent way - a bit quick and dirty, but seems to work well for my use case.

Just thought I'd share if its useful for anyone else.

Example:

"https://www.google.com/"
    .WithPolicy(
        Policy
            .Handle<HttpRequestException>()
            .OrResult<HttpResponseMessage>(r => !r.IsSuccessStatusCode)
            .RetryAsync())
    .GetStringAsync();
cremor commented 9 months ago

@tmenier I just noticed that you removed this issue from the 4.0 plan. Is there a recommended way how to combine Flurl.Http and Polly right now (either in Flurl 3.x or 4.0)? Some time ago I've researched the best way to do this. There were many options and it was hard to understand the differences.

tmenier commented 9 months ago

@cremor I don't really have any official recommendations, unless you can be more specific. It's been discussed on Stack Overflow, you might want to start there. Or take a look at what @SaahilClaypool mentioned above.

cremor commented 9 months ago

I'd like to have a global Polly configuration and then use Flurl "like normal" (e.g. with string extension methods or by getting an IFlurlClientFactory injected). The Flurl client should then automatically use the Polly configuration. It looks like this is not supported by the package from @SaahilClaypool

tmenier commented 8 months ago

@cremor Please ask questions like that on Stack Overflow. This issue is for tracking a feature request. Thanks.

ZandreBroodryk commented 8 months ago

@cremor I think i am replying on the wrong thread for this, but I have had a similar issue, as per @tmenier's reply on https://stackoverflow.com/questions/52272374/set-a-default-polly-policy-with-flurl in v3.x you would override the Factory, which added the desired functionality. however the use of factories have been removed for v4, but the ability to register a DelegatingHandler directly has been provided as middleware, which I mean is what it is.

so the solution to get a similar response is

public class PollyHandler : DelegatingHandler
{
    private readonly IAsyncPolicy<HttpResponseMessage> _policy;

    public PollyHandler(IAsyncPolicy<HttpResponseMessage> policy) {
        _policy = policy;
    }

    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) {
        return _policy.ExecuteAsync(ct => base.SendAsync(request, ct), cancellationToken);
    }
}

and then once on startup you call:

var policy = Policy
    .Handle<HttpRequestException>()
    .OrResult<HttpResponseMessage>(r => !r.IsSuccessStatusCode)
    .RetryAsync(5);
FlurlHttp.Clients.WithDefaults(clientBuilder => clientBuilder.AddMiddleware(() => new PollyHandler (policy)));

I wouldn't confidently say this is the preferred way to achieve the desired outcome, but it seems to be equivalent to the way outlined in stack overflow for v3

tmenier commented 8 months ago

@ZandreBroodryk Well done, I think that approach is perfect. You might not even need to write your own handler, I think you could just use this one? https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.http.policyhttpmessagehandler

That's what practically all of MS's Polly extension methods do: https://github.com/dotnet/aspnetcore/blob/main/src/HttpClientFactory/Polly/src/DependencyInjection/PollyHttpClientBuilderExtensions.cs

jassent commented 8 months ago

[Moving this feature request to here]

Flurl and Flurl.Http are an awesome combo. Unfortunately they are currently incompatible with Polly v8 for implementing retry/resilience logic.

The Flurl.Http AddMiddleware() method works great with Polly v7 but is incompatible with Polly v8.

The issue is that Polly v8 doesn't provide a DelegatingHandler. So I guess this more of a Polly v8 issue than a Flurl.Http issue.

This Sundry.Extensions.Http.Polly seems to get pretty close on how to setup a DelegatingHandler. Refer to the PollyStrategyHttpMessageHandler.cs

Here is a post on StackOverlow with a workaround for getting Flurl.Http to work with Polly v8.

Thank you again for a great library!

jassent commented 8 months ago

This API #proposal for Microsoft.Extensions.Http.Resilience would probably also benefit the Flurl.Http AddMiddleware() method.

tmenier commented 8 months ago

@jassent

This https://github.com/dotnet/extensions/issues/4759 for Microsoft.Extensions.Http.Resilience would probably also benefit the Flurl.Http AddMiddleware() method.

Agreed, and it sounds like they're open to hearing about real-world use cases. Maybe you could voice this one! :)

cremor commented 3 months ago

This API #proposal for Microsoft.Extensions.Http.Resilience would probably also benefit the Flurl.Http AddMiddleware() method.

This was implemented. ResilienceHandler is now public (although it's currently marked as experimental). Here is an example: https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines#resilience-with-static-clients

@tmenier You might want to update the Polly example in the Flurl docs. The package Microsoft.Extensions.Http.Polly which is used there only works with Polly v7 and it looks like it will never be updated to Polly v8. The current MS documentation even calls it deprecated, see https://learn.microsoft.com/en-us/dotnet/core/resilience/ See also dotnet/docs#40841

tmenier commented 3 months ago

@cremor I'd be happy to accept a PR for that update.

LionelVallet commented 2 months ago

Hi, what would be the right way to do it among these three :

services.AddSingleton<IFlurlClientCache>(_ => new FlurlClientCache()
    .WithDefaults(builder => builder
        .AddMiddleware(() => new ResilienceHandler(retryPipeline) { InnerHandler = new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(15) } })));
var socketsHttpHandler = new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(15) };

services.AddSingleton<IFlurlClientCache>(_ => new FlurlClientCache()
    .WithDefaults(builder => builder
        .AddMiddleware(() => new ResilienceHandler(retryPipeline) { InnerHandler = socketsHttpHandler })));
var resilienceHandler = new ResilienceHandler(retryPipeline) { InnerHandler = new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(15) } };

services.AddSingleton<IFlurlClientCache>(_ => new FlurlClientCache()
    .WithDefaults(builder => builder
        .AddMiddleware(() => resilienceHandler)));

What needs to be constructed in the AddMiddleware function? Is there another way to retrieve a SocketsHttpHandler created by Flurl in this function?

Do I even need to define the InnerHandler as in the Microsoft example, or is this precisely the role of AddMiddleware, and the optimal solution would be :

services.AddSingleton<IFlurlClientCache>(_ => new FlurlClientCache()
    .WithDefaults(builder => builder
        .AddMiddleware(() => new ResilienceHandler(retryPipeline))));