Skip to content

Commit

Permalink
HttpClient throws TimeoutException wrapped by TaskCancellationExcepti…
Browse files Browse the repository at this point in the history
…on when request times out (dotnet#2281)

Currently, HttpClient throws the same TaskCancellationException regardless of the request cancellation reason that can be caller's cancellation, all pending request cancellation or timeout. This makes it impossible to handle a request timeout in a way different from all other cases (e.g. special retry logic).

This PR adds a timeout detection logic into HttpClient. It watches for all TaskCancelledExceptions and catches the one caused by timeout. Then, it creates two new exceptions and build a hierarchy. The first is a TimeoutException having its InnerException set to the original TaskCancelledException. The second is a new TaskCancelledException having its InnerException set to that new TimeoutException, but preserving the original stack trace, message and cancellation token. Finally, this top-level TaskCancelledException gets thrown.

Fixes dotnet#21965
  • Loading branch information
alnikola authored Feb 19, 2020
1 parent 993cfd5 commit 7818335
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 10 deletions.
5 changes: 4 additions & 1 deletion src/libraries/System.Net.Http/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -537,4 +537,7 @@
<data name="net_http_qpack_no_dynamic_table" xml:space="preserve">
<value>The HTTP/3 server attempted to reference a dynamic table index that does not exist.</value>
</data>
</root>
<data name="net_http_request_timedout" xml:space="preserve">
<value>The request was canceled due to the configured HttpClient.Timeout of {0} seconds elapsing.</value>
</data>
</root>
49 changes: 44 additions & 5 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.IO;
using System.Net.Http.Headers;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -491,12 +492,14 @@ public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, HttpCompl
CancellationTokenSource cts;
bool disposeCts;
bool hasTimeout = _timeout != s_infiniteTimeout;
long timeoutTime = long.MaxValue;
if (hasTimeout || cancellationToken.CanBeCanceled)
{
disposeCts = true;
cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _pendingRequestsCts.Token);
if (hasTimeout)
{
timeoutTime = Environment.TickCount64 + (_timeout.Ticks / TimeSpan.TicksPerMillisecond);
cts.CancelAfter(_timeout);
}
}
Expand All @@ -512,19 +515,25 @@ public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, HttpCompl
{
sendTask = base.SendAsync(request, cts.Token);
}
catch
catch (Exception e)
{
HandleFinishSendAsyncCleanup(cts, disposeCts);

if (e is OperationCanceledException operationException && TimeoutFired(cancellationToken, timeoutTime))
{
throw CreateTimeoutException(operationException);
}

throw;
}

return completionOption == HttpCompletionOption.ResponseContentRead && !string.Equals(request.Method.Method, "HEAD", StringComparison.OrdinalIgnoreCase) ?
FinishSendAsyncBuffered(sendTask, request, cts, disposeCts) :
FinishSendAsyncUnbuffered(sendTask, request, cts, disposeCts);
FinishSendAsyncBuffered(sendTask, request, cts, disposeCts, cancellationToken, timeoutTime) :
FinishSendAsyncUnbuffered(sendTask, request, cts, disposeCts, cancellationToken, timeoutTime);
}

private async Task<HttpResponseMessage> FinishSendAsyncBuffered(
Task<HttpResponseMessage> sendTask, HttpRequestMessage request, CancellationTokenSource cts, bool disposeCts)
Task<HttpResponseMessage> sendTask, HttpRequestMessage request, CancellationTokenSource cts, bool disposeCts, CancellationToken callerToken, long timeoutTime)
{
HttpResponseMessage response = null;
try
Expand All @@ -548,6 +557,13 @@ private async Task<HttpResponseMessage> FinishSendAsyncBuffered(
catch (Exception e)
{
response?.Dispose();

if (e is OperationCanceledException operationException && TimeoutFired(callerToken, timeoutTime))
{
HandleSendAsyncTimeout(operationException);
throw CreateTimeoutException(operationException);
}

HandleFinishSendAsyncError(e, cts);
throw;
}
Expand All @@ -558,7 +574,7 @@ private async Task<HttpResponseMessage> FinishSendAsyncBuffered(
}

private async Task<HttpResponseMessage> FinishSendAsyncUnbuffered(
Task<HttpResponseMessage> sendTask, HttpRequestMessage request, CancellationTokenSource cts, bool disposeCts)
Task<HttpResponseMessage> sendTask, HttpRequestMessage request, CancellationTokenSource cts, bool disposeCts, CancellationToken callerToken, long timeoutTime)
{
try
{
Expand All @@ -573,6 +589,12 @@ private async Task<HttpResponseMessage> FinishSendAsyncUnbuffered(
}
catch (Exception e)
{
if (e is OperationCanceledException operationException && TimeoutFired(callerToken, timeoutTime))
{
HandleSendAsyncTimeout(operationException);
throw CreateTimeoutException(operationException);
}

HandleFinishSendAsyncError(e, cts);
throw;
}
Expand All @@ -582,6 +604,14 @@ private async Task<HttpResponseMessage> FinishSendAsyncUnbuffered(
}
}

private bool TimeoutFired(CancellationToken callerToken, long timeoutTime) => !callerToken.IsCancellationRequested && Environment.TickCount64 >= timeoutTime;

private TaskCanceledException CreateTimeoutException(OperationCanceledException originalException)
{
return new TaskCanceledException(string.Format(SR.net_http_request_timedout, _timeout.TotalSeconds),
new TimeoutException(originalException.Message, originalException), originalException.CancellationToken);
}

private void HandleFinishSendAsyncError(Exception e, CancellationTokenSource cts)
{
if (NetEventSource.IsEnabled) NetEventSource.Error(this, e);
Expand All @@ -595,6 +625,15 @@ private void HandleFinishSendAsyncError(Exception e, CancellationTokenSource cts
}
}

private void HandleSendAsyncTimeout(OperationCanceledException e)
{
if (NetEventSource.IsEnabled)
{
NetEventSource.Error(this, e);
NetEventSource.Error(this, "Canceled due to timeout");
}
}

private void HandleFinishSendAsyncCleanup(CancellationTokenSource cts, bool disposeCts)
{
// Dispose of the CancellationTokenSource if it was created specially for this request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,14 +593,54 @@ public void CancelAllPending_AllPendingOperationsCanceled(bool withInfiniteTimeo
}
}

[Fact]
public void Timeout_TooShort_AllPendingOperationsCanceled()
[Theory]
[InlineData(HttpCompletionOption.ResponseContentRead)]
[InlineData(HttpCompletionOption.ResponseHeadersRead)]
public void Timeout_TooShort_AllPendingOperationsCanceled(HttpCompletionOption completionOption)
{
using (var client = new HttpClient(new CustomResponseHandler((r, c) => WhenCanceled<HttpResponseMessage>(c))))
{
client.Timeout = TimeSpan.FromMilliseconds(1);
Task<HttpResponseMessage>[] tasks = Enumerable.Range(0, 3).Select(_ => client.GetAsync(CreateFakeUri())).ToArray();
Assert.All(tasks, task => Assert.Throws<TaskCanceledException>(() => task.GetAwaiter().GetResult()));
Task<HttpResponseMessage>[] tasks = Enumerable.Range(0, 3).Select(_ => client.GetAsync(CreateFakeUri(), completionOption)).ToArray();
Assert.All(tasks, task => {
OperationCanceledException e = Assert.ThrowsAny<OperationCanceledException>(() => task.GetAwaiter().GetResult());
TimeoutException timeoutException = (TimeoutException)e.InnerException;
Assert.NotNull(timeoutException);
Assert.NotNull(timeoutException.InnerException);
});
}
}

[Theory]
[InlineData(HttpCompletionOption.ResponseContentRead)]
[InlineData(HttpCompletionOption.ResponseHeadersRead)]
public async Task Timeout_CallerCanceledTokenAfterTimeout_TimeoutIsNotDetected(HttpCompletionOption completionOption)
{
using (var client = new HttpClient(new CustomResponseHandler((r, c) => WhenCanceled<HttpResponseMessage>(c))))
{
client.Timeout = TimeSpan.FromMilliseconds(0.01);
CancellationTokenSource cts = new CancellationTokenSource();
CancellationToken token = cts.Token;
cts.Cancel();
Task<HttpResponseMessage> task = client.GetAsync(CreateFakeUri(), completionOption, token);
OperationCanceledException e = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await task);
Assert.Null(e.InnerException);
}
}

[Theory]
[InlineData(HttpCompletionOption.ResponseContentRead)]
[InlineData(HttpCompletionOption.ResponseHeadersRead)]
public void Timeout_CallerCanceledTokenBeforeTimeout_TimeoutIsNotDetected(HttpCompletionOption completionOption)
{
using (var client = new HttpClient(new CustomResponseHandler((r, c) => WhenCanceled<HttpResponseMessage>(c))))
{
client.Timeout = TimeSpan.FromDays(1);
CancellationTokenSource cts = new CancellationTokenSource();
Task<HttpResponseMessage> task = client.GetAsync(CreateFakeUri(), completionOption, cts.Token);
cts.Cancel();
OperationCanceledException e = Assert.ThrowsAny<OperationCanceledException>(() => task.GetAwaiter().GetResult());
Assert.Null(e.InnerException);
}
}

Expand Down

0 comments on commit 7818335

Please sign in to comment.