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);
        }
    }
}

@bitm0de I don't find it more convenient myself, but if you prefer that approach, by all means go ahead and use it 😉

How would you do that for PostAsync() method ?

@adydeshmukh there's nothing special to do for PostAsync. The handler applies to all methods.

Thanks, I found this helpful.

I thought about the inherit approach but then I must know all the places HttpWebRequest is created but on the other hand, only enrolling requests where I want this timeout behavior could be handy at instantiation.

Use ConfigureAwait(false) is my only suggestion since it highly unlikely the thread this is processed on matters.

On the subject of inheriting from HttpRequestMessage, when the ASP.NET Core documentation discusses delegating handlers here, it says:

Use one of the following approaches to share per-request state with message handlers:

• Pass data into the handler using HttpRequestMessage.Properties.
• Use IHttpContextAccessor to access the current request.
• Create a custom AsyncLocal storage object to pass the data.

HttpRequestMessage.Properties is explicitly named as a recommended way to pass per-request state into the handler, while subclassing HttpRequestMessage is not.

On the subject of inheriting from HttpRequestMessage, when the ASP.NET Core documentation discusses delegating handlers here, it says:

Use one of the following approaches to share per-request state with message handlers:

• Pass data into the handler using HttpRequestMessage.Properties.
• Use IHttpContextAccessor to access the current request.
• Create a custom AsyncLocal storage object to pass the data.

HttpRequestMessage.Properties is explicitly named as a recommended way to pass per-request state into the handler, while subclassing HttpRequestMessage is not.

Reading the very first part:

Use one of the following approaches to share per-request state with message handlers

This tells me that it's listing suggestions for sharing per-request state with message handlers. A timeout doesn't need to share anything with the actual message handlers in this case. The implementation here is using a CancellationToken. Context is key here...

A request timeout is not really a request state that needs to be associated with the message handlers. If the class wasn't meant to be inherited from, it wouldn't be designed in such a way that would indicate otherwise (i.e. protected virtual void Dispose(bool disposing)). If you're designing for extensibility (which is the end goal here), polluting that Dictionary with timeout information might not be preferred either; and you also have no control over the mutability of the timeout in that Dictionary... Dictionary's are IEnumerable<T>, which means you'd potentially have to add a special case in your code to ignore the timeout key/value pair if you needed to implement code that enumerates all of the key/value pairs to do certain things with them. With inheritance, you don't have to worry about doing anything differently if you wanted to extend this functionality. If you speculatively use intuition to weigh the benefits and disadvantages here, I would say extensibility via inheritance would make sense. If you feel otherwise, what disadvantages is there?

Omissions in the Microsoft documentation don't inherently mean that alternative ways are wrong (even though I think you might've misunderstood the context of the listed suggestions). If you've read enough documentation, you would see that there are even syntax errors on some of those pages. (Not saying the documentation is wrong in this case, but there's plenty of potential for the documentation to miss certain things, in addition to being wrong in other cases.)

In .NET 5.0, Properties is marked as Obsolete as well in favor of the Options property: https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httprequestmessage.properties?view=net-5.0#System_Net_Http_HttpRequestMessage_Properties

A timeout doesn't need to share anything with the actual message handlers in this case.

I'm confused what you mean by this. The timeout value is state that can vary by request, and the value of that is shared with the TimeoutHandler. That looks to me like a timeout sharing something with an actual message handler. Are you reading "share" as describing something more specific than merely making certain information available?

In .NET 5.0, Properties is marked as Obsolete as well in favor of the Options property

Yes, but Options is a key-value structure just like Properties, just with stronger typing. If you check the issue linked to the pull request that added it to .NET (dotnet/runtime#34168, dotnet/runtime#39182), the first mention of the type in the discussion is this:

Mechanics. It seems we all like the concept of a loose property bag over message that a handler can inspect to perform request specific operations. However, we don't like the loose typing of HttpRequestMessage.Properties which is IDictionary<string, object>. It seems the easiest short term solution is an extension methods over HttpRequestMessage like SetRequestCache/GetRequestCache that will set/get the value in the property bag. Alternatively, we could expose a new type, such as HttpMessageOptions that people can pass around and that we can be stored in HttpRequestMessage.Properties. This way, libraries can accept an HttpMessageOptions and use that in case they are constructing HTTP messages themselves.

This looks to me like extension methods like those in the example are being described as the "short term" solution until the better-typed Options is implemented, and the general strategy of "toss it in the property bag" is being viewed positively in both cases.

A timeout doesn't need to share anything with the actual message handlers in this case.

I'm confused what you mean by this. The timeout value is state that can vary by request, and the value of that is shared with the TimeoutHandler. That looks to me like a timeout sharing something with an actual message handler. Are you reading "share" as describing something more specific than merely making certain information available?

It is state in terms of the request, but read what Microsoft wrote (emphasized): "Use one of the following approaches to share per-request state WITH message handlers"
My previous statement: "A request timeout is not really a request state that needs to be associated with the message handlers"

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. My inheritance approach also doesn't inherently become obsolete with the change in .NET 5.0. To get a timeout you don't necessarily need to share the timeout state across message handlers though, and my own implementation allows it to be somewhat separate.

edit: Perhaps there is just a discrepancy between how we're viewing the word "share" as you hinted at in your prior comment.

In .NET 5.0, Properties is marked as Obsolete as well in favor of the Options property

Yes, but Options is a key-value structure just like Properties, just with stronger typing. If you check the issue linked to the pull request that added it to .NET (dotnet/runtime#34168, dotnet/runtime#39182), the first mention of the type in the discussion is this:

I was referencing this to demonstrate how my solution is future proof based on these changes.

Mechanics. It seems we all like the concept of a loose property bag over message that a handler can inspect to perform request specific operations. However, we don't like the loose typing of HttpRequestMessage.Properties which is IDictionary<string, object>. It seems the easiest short term solution is an extension methods over HttpRequestMessage like SetRequestCache/GetRequestCache that will set/get the value in the property bag. Alternatively, we could expose a new type, such as HttpMessageOptions that people can pass around and that we can be stored in HttpRequestMessage.Properties. This way, libraries can accept an HttpMessageOptions and use that in case they are constructing HTTP messages themselves.

This looks to me like extension methods like those in the example are being described as the "short term" solution until the better-typed Options is implemented, and the general strategy of "toss it in the property bag" is being viewed positively in both cases.

This is all fair, but in the context of the requirement for sharing state between message handlers, a timeout doesn't need to as I mentioned earlier. (i.e. The underlying solution here is based on exceptions.)

There are hints towards the dislike of the loose typing with the initial property as well. In my case that isn't an issue at all with extending the class. My overall view is that this should be something that you can apply to any request by default. (Additionally, unless you have a really unique key name, an external implementer runs the risk of accidentally overwriting that key with a new value. I think the extensibility in my case helps mitigate a lot of problems like this for a universal approach.)

I'm open to any pitfalls though if you can think of something that I'm missing. (To clarify, I'm also not saying to avoid Properties/Options entirely. I use it for my serialization/deserialization and authentication pipelines.)

In .NET 5.0, Properties is marked as Obsolete as well in favor of the Options property

Should be noted, in .NET 5.0, the thrown exception carries an InnerException of TimeoutException. If you need per request, I think you still end up with something like this though.

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.

This is a lifesaver. I did have to tweak it to work on a mobile client. The error thrown is InvalidOperationException. Also, .net8.0 has obsoleted request.Properties, so I modified it to use request.Options. Other than that, it works great and I only have to change the timeout in the few places where I need to wait a little longer.