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

feature: Have the possibility to setup Polly Context #801

Open
andrekiba opened this issue Dec 1, 2019 · 5 comments
Open

feature: Have the possibility to setup Polly Context #801

andrekiba opened this issue Dec 1, 2019 · 5 comments

Comments

@andrekiba
Copy link

Hello and thanks in advance!
What about the case in which Polly and Refit are used with HttpClientFactory?
For example: is there a way to setup the Polly Context before each call with Refit?
https://github.com/App-vNext/Polly/wiki/Keys-And-Context-Data
I didn't find a way to manage this.
In Controllers for example you inject the refit-generated interface (and it is amazing) but at this point you can't setup the Context because you can't access the underlying http request.
Polly provides the request.SetPolicyExecutionContext(context); but we can't use it.
https://github.com/App-vNext/Polly/wiki/Polly-and-HttpClientFactory#use-case-exchanging-information-between-policy-execution-and-calling-code

@james-s-tayler
Copy link
Contributor

james-s-tayler commented Nov 24, 2020

I think once #1001 is merged you could actually do this!

Looking at the source code for request.SetPolicyExecutionContext(context) behind the scenes on the HttpRequestMessage it's simply populating request.Properties["PolicyExecutionContext"] = pollyContext, so all you would need to do is add [Property("PolicyExecutionContext")] Polly.Context pollyContext to your method signature and then pass it new Polly.Context()

@turacma
Copy link

turacma commented May 24, 2022

It seems like this suggestion has made it into the docs as a recommended way to pass the Polly Context, but has this been confirm to actually work? I've been trying to set this up to pass an ILogger to my policies but haven't been able to get it to actually work.,

Even though I've added the property to my method signature:

Task<UserSubscriptions> GetUserSubscriptions(string userId, [Property("PollyExecutionContext")] Polly.Context context);

And create a new context when executing the method:

private Polly.Context GetPollyContext() {
        return new Context($"GetSomeData-{Guid.NewGuid()}", new Dictionary<string, object>
        {
            { "polly-logger", logger }
        });
}

restClient.GetUserSubscriptions(userId.UserIdString, GetPollyContext());

When I debug the policy the context that I see when the action is executed is an new empty context and not the one I created with the attached logger.

@james-s-tayler
Copy link
Contributor

After some digging it turns out you've found a bug in the docs. I'll submit a PR for it later on. The correct key is "PolicyExecutionContext". Polly and Policy are so similar I must have just misread or mistyped it when adding it to the docs. It does work though. I replicated it in a unit test and also included advice on how to better accomplish the same thing through the use of a DelegatingHandler without passing it through the [Property] attribute if you don't need it to be initialized with dynamic content.

using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Polly;
using Refit;
using Xunit;

namespace Refit.Bug.Repro.Tests
{
    public class RefitPollyTests    
    {
        public interface IHttpStatus
        {
            //property key "PollyExecutionContext" stated in the docs is wrong - this is a bug in the docs :(
            [Get("/{statusCode}")]
            Task<ApiResponse<string>> GetStatusCodeWithPollyExecutionContextProperty([Query] int statusCode, [Property("PollyExecutionContext")] Context pollyContext);
        
            //the correct property key is "PolicyExecutionContext"
            [Get("/{statusCode}")]
            Task<ApiResponse<string>> GetStatusCodeWithPolicyExecutionContextProperty([Query] int statusCode, [Property("PolicyExecutionContext")] Context pollyContext);
        
            //this one leverages a DelegatingHandler to populate the Polly.Context instead,
            //which is a better and much cleaner option if the content of the Polly.Context doesn't _require_ dynamic content only known at runtime
            [Get("/{statusCode}")]
            Task<ApiResponse<string>> GetStatusCodeWithPollyContextInjected([Query] int statusCode);
        }
        
        private const string Logger = nameof(Logger);

        [Fact]
        public async Task    GivenPropertyNameOfPollyExecutionContext_WhenRetryPolicyInvoked_ThenThrowsKeyNotFoundException()
        {
            //arrange
            var services = new ServiceCollection();

            services.AddSingleton<ILogger, TestLogger>();
            services.AddRefitClient<IHttpStatus>()
                .ConfigureHttpClient(client =>
                {
                    client.BaseAddress = new Uri("http://httpstat.us/");
                })
                .AddTransientHttpErrorPolicy(builder => builder.RetryAsync((result, retryCount, context) =>
                {
                    var logger = context[Logger] as ILogger;
                    logger!.LogInformation($"retry_count={retryCount}");
                }));
            var serviceProvider = services.BuildServiceProvider();

            var logger = serviceProvider.GetService<ILogger>();
            var refitClient = serviceProvider.GetService<IHttpStatus>();
            
            var pollyContext = new Context();
            pollyContext.Add(Logger, logger);
            
            //act
            Func<Task> refitClientInvocation = refitClient.Awaiting(
                client => client.GetStatusCodeWithPollyExecutionContextProperty(StatusCodes.Status503ServiceUnavailable, pollyContext));
            
            //assert
            await refitClientInvocation.Should()
                .ThrowAsync<KeyNotFoundException>()
                .WithMessage("The given key 'Logger' was not present in the dictionary.");
        }
    
        [Fact]
        public async Task GivenPropertyNameOfPolicyExecutionContext_WhenRetryPolicyInvoked_ThenLogsRetryCount()
        {
            //arrange
            var services = new ServiceCollection();

            services.AddSingleton<ILogger, TestLogger>();
            services.AddRefitClient<IHttpStatus>()
                .ConfigureHttpClient(client =>
                {
                    client.BaseAddress = new Uri("http://httpstat.us/");
                })
                .AddTransientHttpErrorPolicy(builder => builder.RetryAsync((result, retryCount, context) =>
                {
                    var logger = context[Logger] as ILogger;
                    logger!.LogInformation($"retry_count={retryCount}");
                })); 
            var serviceProvider = services.BuildServiceProvider();

            var logger = serviceProvider.GetService<ILogger>();
            var refitClient = serviceProvider.GetService<IHttpStatus>();
            
            var pollyContext = new Context();
            pollyContext.Add(Logger, logger);
        
            //act
            var result = await refitClient.GetStatusCodeWithPolicyExecutionContextProperty(StatusCodes.Status503ServiceUnavailable, pollyContext);
            
            //assert
            result.StatusCode.Should().Be(HttpStatusCode.ServiceUnavailable);
            var testLogger = logger as TestLogger;
            testLogger.Logs.Count.Should().Be(1);
            testLogger.Logs.Should().Contain("retry_count=1");
        }
        
        [Fact]
        public async Task    GivenPollyContextInjectedByDelegatingHandler_WhenRetryPolicyInvoked_ThenLogsRetryCount()
        {
            //arrange
            var services = new ServiceCollection();

            services.AddSingleton<ILogger, TestLogger>();
            services.AddTransient<PollyContextInjectingDelegatingHandler>();
            services.AddRefitClient<IHttpStatus>()
                .ConfigureHttpClient(client =>
                {
                    client.BaseAddress = new Uri("http://httpstat.us/");
                })
                //order matters here
                .AddHttpMessageHandler<PollyContextInjectingDelegatingHandler>()
                .AddTransientHttpErrorPolicy(builder => builder.RetryAsync((result, retryCount, context) =>
                {
                    var logger = context[Logger] as ILogger;
                    logger!.LogInformation($"retry_count={retryCount}");
                }));
            var serviceProvider = services.BuildServiceProvider();

            var logger = serviceProvider.GetService<ILogger>();
            var refitClient = serviceProvider.GetService<IHttpStatus>();

            //act
            var result = await refitClient.GetStatusCodeWithPollyContextInjected(StatusCodes.Status503ServiceUnavailable);
            
            //assert
            result.StatusCode.Should().Be(HttpStatusCode.ServiceUnavailable);
            var testLogger = logger as TestLogger;
            testLogger.Logs.Count.Should().Be(1);
            testLogger.Logs.Should().Contain("retry_count=1");
        }

        public class PollyContextInjectingDelegatingHandler : DelegatingHandler
        {
            //realistically you would probably use a logger factory or ILogger<T> here
            //but just showing you how it can be accomplished through DI and a DelegatingHandler more cleanly
            private readonly ILogger _logger;
            
            public PollyContextInjectingDelegatingHandler(ILogger logger)
            {
                _logger = logger;
            }
            
            protected override async Task<HttpResponseMessage> SendAsync(
                HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
            {
                var pollyContext = new Context();
                pollyContext.Add(Logger, _logger);
                request.SetPolicyExecutionContext(pollyContext);
                
                return await base.SendAsync(request,    cancellationToken).ConfigureAwait(false);
            }
        }
        
        public class TestLogger : ILogger
        {
            public List<string> Logs { get; } = new ();
            
            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state,    Exception exception, Func<TState, Exception, string> formatter)
                {
                Logs.Add(formatter.Invoke(state, exception));
            }

            public bool IsEnabled(LogLevel logLevel)
            {
                return true;
            }

            public IDisposable BeginScope<TState>(TState state)
            {
                return default;
            }
        }
    }
}

@turacma
Copy link

turacma commented May 25, 2022

@james-s-tayler fantastic! Thank you so much fixing the doc bug and pointing out the delegate injection, which I agree is much better for my use case.

james-s-tayler added a commit to james-s-tayler/refit that referenced this issue May 25, 2022
A bug report came in via an old issue reactiveui#801 that highlighted a bug in the docs. I've fixed the doc bug and added some guidance on how better to support a common use case and outlined in what case it's appropriate to pass the `Polly.Context` via the `[Property]` attribute.
@james-s-tayler
Copy link
Contributor

@clairernovotny once #1366 is merged we can close this issue as it's fully supported now and the docs are clear on when/how to use it.

Thanks @turacma for raising this.

glennawatson pushed a commit that referenced this issue Apr 12, 2023
A bug report came in via an old issue #801 that highlighted a bug in the docs. I've fixed the doc bug and added some guidance on how better to support a common use case and outlined in what case it's appropriate to pass the `Polly.Context` via the `[Property]` attribute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants