Runtime: HttpClientFactory doesn't respect lifetime of custom message handlers

[DiamondDragon]

Here is example of code i use:

            services.AddHttpClient(serviceName.ToLowerInvariant(), client =>
                {
                    client.BaseAddress = new Uri(serviceEndpoint.BaseAddress);
                    client.Timeout = TimeSpan.FromSeconds(serviceEndpoint.Timeout);

                    client.DefaultRequestHeaders.Accept.Clear();
                    client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(MessagePackSerializer.ContentType));
                    client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
                })
                .AddHttpMessageHandler<CorrelationIdHandler>()
                .AddHttpMessageHandler<RequestResponseLoggingHandler>() 
                .AddHttpMessageHandler<TrustedClientAuthenticationHandler>();

Handlers are registered as follows:

            services.AddTransient<CorrelationIdHandler>();
            services.AddTransient<RequestResponseLoggingHandler>();
            services.AddTransient<TrustedClientAuthenticationHandler>();

When first request is sent by HttpClient whole processing pipeline is created. When second request is sent neither of handlers is created. Handlers are already created as part of first request processing. After two minutes (e.g. default lifetime of handler) handlers are recreated.

Current behavior causes problem when handlers resolve dependencies from container. These transient dependencies become "locked" by handler instances. For example this may cause issue if handler injects authentication headers based on ASP.NET Core request principal. E.g. ClaimsPrincipal of first request is used for subsequent requests.

[DiamondDragon]

Official documentation says nothing about described side effects:

https://docs.microsoft.com/en-us/dotnet/standard/microservices-architecture/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests#httpclient-lifetimes

[rynowak]

Yes. The factory manages the lifetimes of all handlers that get created. I wouldn't suggest trying to use per-request services to share state.

I'll follow up on improving the documentation in this area.

You can use use IHttpContextAccessor if you want to register a handler that has access to the current ASP.NET Core request. Does this help?

[DiamondDragon]

@rynowak to my mind current design significantly limits possible use cases for message handlers. As workaround i'm going to create HttpClient explicitly and specify all message handlers as constructor parameters and use instance of client to handle single request. As result I'm not going to benefit from described improvements.

I think it would be beneficial if HttpClientFactory could work as follows:

• Continue to manage lifetime of HttpMessageHandler as it does now. E.g. reuses it for two minutes by default in order to manage connections efficiently.
• Create custom handlers per each request.
• Create pipeline using cached HttpMessageHandler and newly created custom handlers.

I think this behavior would be in sync with current documentation and would not surprise developers. On other hand it might be difficult to implement it because handlers are provided as constructor parameter of HttpClient and there is no way to find out lifetime of this object (it may be used for more than one request).

[DiamondDragon]

@rynowak after some thinking about IHttpContextAccessor I came to the following solution which may work only when IHttpContext is available:
Before:

    public class TrustedClientAuthenticationHandler : DelegatingHandler
    {
        private readonly ITrustedClientAuthenticationTokenBuilder tokenBuilder;
        private readonly IMyClaimsPrincipal principal;

        private const string AuthType = TrustedClientAuthenticationOptions.AuthType;

        public TrustedClientAuthenticationHandler(ITrustedClientAuthenticationTokenBuilder tokenBuilder, IMyClaimsPrincipal principal)
        {
            this.tokenBuilder = tokenBuilder;
            this.principal = principal;
        }

        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            var dateSent = DateTime.UtcNow;
            request.Headers.Date = dateSent;

            var authParams = tokenBuilder.Build(dateSent, principal.Claims);
            if (authParams == null)
            {
                var task = new TaskCompletionSource<HttpResponseMessage>();
                task.SetResult(new HttpResponseMessage(HttpStatusCode.Unauthorized));

                return task.Task;
            }

            request.Headers.Authorization = new AuthenticationHeaderValue(AuthType, authParams);
            return base.SendAsync(request, cancellationToken);
        }
    }

After

    public class TrustedClientAuthenticationHandler : DelegatingHandler
    {
        private readonly IHttpContextAccessor httpContext;

        private const string AuthType = TrustedClientAuthenticationOptions.AuthType;

        public TrustedClientAuthenticationHandler(IHttpContextAccessor httpContext)
        {
            this.httpContext = httpContext ?? throw new ArgumentNullException(nameof(httpContext));
        }

        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            var tokenBuilder = GetService<ITrustedClientAuthenticationTokenBuilder>();
            var principal = GetService<IMyClaimsPrincipal>();

            var dateSent = DateTime.UtcNow;
            request.Headers.Date = dateSent;

            var authParams = tokenBuilder.Build(dateSent, principal.Claims);
            if (authParams == null)
            {
                var task = new TaskCompletionSource<HttpResponseMessage>();
                task.SetResult(new HttpResponseMessage(HttpStatusCode.Unauthorized));

                return task.Task;
            }

            request.Headers.Authorization = new AuthenticationHeaderValue(AuthType, authParams);
            return base.SendAsync(request, cancellationToken);
        }

        private T GetService<T>()
        {
            return (T)httpContext.HttpContext.RequestServices.GetService(typeof(T));
        }
    }

In other cases:

• Background workers
• Message queue handlers

This solution will not work because there might be no way to get access to scoped service provider. E.g. you might not be able to use anything similar to IHttpContextAccessor.

[rynowak]

I think it would be beneficial if HttpClientFactory could work as follows:

Thanks, we've gotten this feedback from a few others well. This will not change for 2.2 but we will consider this in the future. Doing a change like this will involve a lot more technical complexity both inside and outside the HttpClientFactory library, and will come with a few more 'gotcha' things that users have to know.

I think this behavior would be in sync with current documentation and would not surprise developer

As a personal note, it's a much safer bet to assume that a library does not provide guarantees that are not listed. We're going to update the docs to make this super clear.

Background workers
Message queue handlers
This solution will not work because there might be no way to get access to scoped service provider. E.g. you might not be able to use anything similar to IHttpContextAccessor.

For these cases, if this worked the way you want with unit of work-scoped-services you'd still be responsible for setting up the context you want - ie, defining what the unit of work is and managing a DI scope.

If you want the ability to flow 'ambient' data into a message handler without any direct code calling into it you could also use your own async local for IMyCustomClaimsPrincipal, which is what [HttpContextAccessor](https://github.com/aspnet/HttpAbstractions/blob/master/src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs#L8) andCultureInfo.CurrentCulture` use.

[DiamondDragon]

@rynowak i was thinking about injecting custom context accessors into containers for message handler and background worker cases. This is still doable but looks like a lot of extra work just to use message handlers for HttpClient. It doesn't look like any other code in our infrastructure needs it.

Thank you for your feedback! Looking forward to see changes in IHttpHandlerFactory which simplify development of custom message handlers.

[msftbot[bot]]

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.