Skip to content

Instantly share code, notes, and snippets.

@thomaslevesque
Last active December 1, 2023 13:08
  • Star 54 You must be signed in to star a gist
  • Fork 13 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save thomaslevesque/b4fd8c3aa332c9582a57935d6ed3406f to your computer and use it in GitHub Desktop.
TimeoutHandler for smarter timeout handling with HttpClient

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
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;
}
}
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;
}
}
}
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
Copy link

bitm0de commented Jan 27, 2021

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.)

@tvanbaak
Copy link

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.

@bitm0de
Copy link

bitm0de commented Jan 28, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment