TimeoutHandler for smarter timeout handling with HttpClient

0. README.md

TimeoutHandler for smarter timeout handling with HttpClient

This code illustrates the blog article Better timeout handling with HttpClient.

Key features:

• control the timeout per request, rather than globally for all requests
• throw a more sensible exception (TimeoutException) when a timeout occurs, instead of the usual OperationCanceledException

1. HttpRequestExtensions.cs

public static class HttpRequestExtensions
{
    private const string TimeoutPropertyKey = "RequestTimeout";
    
    public static void SetTimeout(this HttpRequestMessage request, TimeSpan? timeout)
    {
        if (request == null) throw new ArgumentNullException(nameof(request));
        request.Properties[TimeoutPropertyKey] = timeout;
    }
    
    public static TimeSpan? GetTimeout(this HttpRequestMessage request)
    {
        if (request == null) throw new ArgumentNullException(nameof(request));
        if (request.Properties.TryGetValue(TimeoutPropertyKey, out var value) && value is TimeSpan timeout)
            return timeout;
        return null;
    }
}

2. TimeoutHandler.cs

public class TimeoutHandler : DelegatingHandler
{
    public TimeSpan DefaultTimeout { get; set; } = TimeSpan.FromSeconds(100);
    
    protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        using (var cts = GetCancellationTokenSource(request, cancellationToken))
        {
            try
            {
                return await base.SendAsync(request, cts?.Token ?? cancellationToken);
            }
            catch(OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
            {
                throw new TimeoutException();
            }
        }
    }
    
    private CancellationTokenSource GetCancellationTokenSource(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        var timeout = request.GetTimeout() ?? DefaultTimeout;
        if (timeout == Timeout.InfiniteTimeSpan)
        {
            // No need to create a CTS if there's no timeout
            return null;
        }
        else
        {
            var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
            cts.CancelAfter(timeout);
            return cts;
        }
    }
}

3. Test.cs

async Task TestAsync()
{
    var handler = new TimeoutHandler
    {
        DefaultTimeout = TimeSpan.FromSeconds(10),
        InnerHandler = new HttpClientHandler()
    };

    using (var cts = new CancellationTokenSource())
    using (var client = new HttpClient(handler))
    {
        client.Timeout = Timeout.InfiniteTimeSpan;
        
        var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost:8888/");

        // Uncomment to test per-request timeout
        //request.SetTimeout(TimeSpan.FromSeconds(5));
        
        // Uncomment to test that cancellation still works properly
        //cts.CancelAfter(TimeSpan.FromSeconds(2));
        
        using (var response = await client.SendAsync(request, cts.Token))
        {
            Console.WriteLine(response.StatusCode);
        }
    }
}

I wasn't disputing whether it is "state" or not -- I was explaining that it isn't something that needs to be shared among the message handlers, which is the context for the suggestions listed.

I'm sorry, I still don't understand your point on this. If the value of the timeout is not shared with the message handler, then it cannot kill the request once the timeout is reached. This isn't even a difference between the inheritance approach and the property bag approach, if, I assume, you mean the same approach as the user above who first asked about inheritance, that we "check if the HttpRequestMessage is of that derived type and grab the timeout from there". In either case, we need to do what I mean by "share": expose information that belongs to the request to the message handler, so it can act accordingly.

Perhaps you can share some code that clarifies what you're saying about having a request-specific timeout without the handler having access to what that timeout is, because I haven't been able to think of a way to do that that makes sense.

My overall view is that this should be something that you can apply to any request by default.

I would think this is a point in favor of the property bag. If I've already subclassed HttpRequestMessage to add some other property, or received an HttpRequestMessage from a third-party library that I can't modify or inherit, then the inheritance approach doesn't allow me to add timeouts to this request. On the other hand, every subclass of HttpRequestMessage has the same property bag, so I can add any number of property-based extensions and support them by composing the HttpClientHandlers that handle them. It's similar to a Python mixin class, and arguably the more extensible option. Problems like having to slightly update the property bag syntax for .NET 5.0 or picking a sufficiently unique string key are trivial to solve in comparison.

My overall view is that this should be something that you can apply to any request by default.

I would think this is a point in favor of the property bag. If I've already subclassed HttpRequestMessage to add some other property, or received an HttpRequestMessage from a third-party library that I can't modify or inherit, then the inheritance approach doesn't allow me to add timeouts to this request. On the other hand, every subclass of HttpRequestMessage has the same property bag, so I can add any number of property-based extensions and support them by composing the HttpClientHandlers that handle them. It's similar to a Python mixin class, and arguably the more extensible option. Problems like having to slightly update the property bag syntax for .NET 5.0 or picking a sufficiently unique string key are trivial to solve in comparison.

Why not just subclass the derived class at that point then? There will always be an argument for both sides IMHO. Changing the identifier of a derived class is just as trivial in most cases. Extending the initial class also makes it easy for any further derived classes to utilize the extended Timeout properties (without having to worry about how it was implemented in the first place), but it also grants you the ability to hide certain parts of the implementation... You can't do any of this by exposing the required information within that Dictionary, and so you leave the opportunity for external implementation to break it. (This also ties into what I was referring to above -- you can hide whatever implementation you want by using different access modifiers, but ultimately you have a lot more control in this regard via inheritance. You don't need to mark the derived class as sealed.)

Why not just subclass the derived class at that point then?

That couples the timeout functionality to the other property that the subclass implemented, which may not be desirable. Alternatively, I may not even be able to subclass it if it came from a third-party library that sealed it. But any subclass of HttpRequestMessage will have the property bag.

Why not just subclass the derived class at that point then?

That couples the timeout functionality to the other property that the subclass implemented, which may not be desirable. Alternatively, I may not even be able to subclass it if it came from a third-party library that sealed it. But any subclass of HttpRequestMessage will have the property bag.

Yes, but if you’re debating use cases here then there’s also a case to be made for the mutability of the data in that dictionary being undesirable as well. If that was the case with a third party lib then it might also be the case that they hide that property to avoid external fiddling as well anyways. Thus, third party libs can introduce problems with your ability to access and use the internal dictionary just as equally as it can affect your ability to inherit from a class... There’s no guarantee that you’d have access to these things with a third party lib depending on the functionality it provides (i.e. Open-closed principle might hide this property but expose other things for extensibility). As such, I think this discussion should omit the unknown.