[Feature request]: Resilience Execution

[caigen]

Is your feature request related to a specific problem? Or an existing feature?

Polly already supports 2 main core concepts:

• Resilience Policy
• Resilience Pipeline

What I want to is:

• Add Resilience Execution Concept & Abstraction for reducing effort when we write Lamba or callback with [Resilience Pipeline] (https://www.pollydocs.org/pipelines/index.html).

I used it somewhere but wanted to contribute it back to Polly as I think it should be general and after it is added in polly, user can create resilience business logic easily & quickly. It can also be used for non-http related resilience.

Sample usage:

        // 1. Config Hedging & Timeout
        ResiliencePipelineBuilder<HttpResponseMessage> builder =
            new ResiliencePipelineBuilder<HttpResponseMessage>();
        builder = builder.AddHedging(
            new Polly.Hedging.HedgingStrategyOptions<HttpResponseMessage>
            {
                Delay = TimeSpan.FromSeconds(2),
                MaxHedgedAttempts = 1,
                ShouldHandle = async args =>
                {
                    return await Task.FromResult(args.Outcome.Result?.IsSuccessStatusCode ?? true);
                },
            });
        builder.AddTimeout(TimeSpan.FromSeconds(25));

        // 2. Build pipeline
        ResiliencePipeline<HttpResponseMessage> pipeline = builder.Build();

        // 3. Config ResilienceInvoker with multiple HttpClients
        FakeHttpClient fakeHttpClientA = new FakeHttpClient("A", 20);
        FakeHttpClient fakeHttpClientB = new FakeHttpClient("B", 5);

        // 4. runtime
        using HttpRequestMessage httpRequestMessage = new HttpRequestMessage();
        CancellationToken cancellationToken = new CancellationToken(false);

        // Invokers per request or we may have thread safety issue
        ResilienceInvoker<HttpResponseMessage, HttpRequestMessage> resilienceInvoker = new ResilienceInvoker<HttpResponseMessage, HttpRequestMessage>(
            pipeline,

            // Create one composite invoker internally
            new List<IInvoker<HttpResponseMessage, HttpRequestMessage>>
            {
                // Add multiple HttpClient here
                fakeHttpClientA,
                fakeHttpClientB
            });
        HttpResponseMessage httpResponseMessage = await resilienceInvoker.ExecuteAsync(httpRequestMessage, cancellationToken);
        Assert.Equal(HttpStatusCode.OK, httpResponseMessage.StatusCode);
        Assert.True(fakeHttpClientA.CalledNumber == 1 || fakeHttpClientB.CalledNumber == 1);

Describe the solution you'd like

/// <summary>
/// IInvoker interface: work together with Polly ResiliencePipeline as callback
/// See: https://www.pollydocs.org/pipelines/index.html .
/// </summary>
/// <typeparam name="TResult">The type of the result.</typeparam>
/// <typeparam name="TInput">The type of the input.</typeparam>
public interface IInvoker
    <TResult, TInput>
{
    /// <summary>
    /// Execute the request.
    /// </summary>
    /// <param name="request">The request.</param>
    /// <param name="cancellationToken">The cancellationToken.</param>
    /// <returns>The result.</returns>
    Task<TResult> ExecuteAsync(TInput request, CancellationToken cancellationToken);
}

classDiagram
    IInvoker <|-- CompositeInvoker
    IInvoker <|-- ResilienceInvoker
    class IInvoker{
      -//Execution abstraction
      -ExecuteAsync()
    }
    class CompositeInvoker{
      -IList _invokers
      +ExecuteAsync()
    }
    class ResilienceInvoker{
      +ResiliencePipeline _resiliencePipeline
      +CompositeInvoker _compositeInvoker
      +ExecuteAsync()
    }

Additional context

IInvoker could be public.
CompositeInvoker could reuse current Composite infra in Polly Core.

No response

[martincostello]

Thanks for opening this issue.

At first glance I'm not sure this is something we'd want.

Usage in the manner described is creating more overhead through the addition objects and need to enumerate the invocations, and Polly v8 was designed to be low allocation, so this would go counter to that.

Also the interface only allows for invocations with one specific signature, and doesn't have the flexibility of the additional overloads the abstract strategies have to accept a context and /or state as well as a CancellationToken.

Can you provide some further details about the motivating factors on why this is something you think would be a good addition to the core library? It just looks like syntactic sugar specific to how you happen to use Polly in your own application.

[caigen]

Thanks for reviewing this.

1. For current ResiliencePipeline, I found it is a little bit complicated when I tried to combine State with Callback together. I want something trivial or near one line.

2. In my case, it is one high availability usage case: e.g. when 1st service/solution has some issue, fall back to 2nd service/solution. The service could be http service or not. But most are http cases.

3. yes, it is sugar also not. I want this sugar is stable and could make my code simpler and easily to maintain. And if is OO design, we can easily extend it also.
   It may be something nice to have. But if we have many users who need to write this similar thing again and again, we can provide one stable version for users to select to use it.
   E.g. Company A may need to write it. Company B may need to write it again. Team A and Team B in Company C may need to write it again.

4. The interface or specific signature is just for enhancement not replacing current flexibility.

Not sure if this interface design is right approach, but the key motivation & goal from me is to Combine Execution & ResiliencePipeline & ResiliencePolicy easily.

[caigen]

Sharing some background about why I name it as IInvoker and provide ExecuteAsync for reviewing:

• It provides one more generic abstraction like System.Net.Http.HttpMessageInvoker, not just for SendAsync but also for similar operations as ResiliencePipeline is generic enough for other operations also.

• At same time, there is no IHttpMessageInvoker from .net currently for extension.

[martincostello]

I'm afraid I'm still not convinced of the need to have this baked into the core library.

Maybe a more fully-fleshed out example (by which I mean something we could actually run) of this being used by real-world code would help with the sales pitch 😄.

[caigen]

Yes, let me make one draft PR on samples first to demo. Then we can review to see if it is something we should provide in lib.

[github-actions]

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

[github-actions]

This issue was closed because it has been inactive for 14 days since being marked as stale.