Skip to content

Commit

Permalink
Fix the token included by HttpClient.HandleFailure (#53133)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stephentoub
stephentoub committed May 24, 2021
1 parent 5613628 commit 42af85b
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 20 deletions.
26 changes: 20 additions & 6 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,17 +583,31 @@ private void HandleFailure(Exception e, bool telemetryStarted, HttpResponseMessa

Exception? toThrow = null;

if (e is OperationCanceledException oce && !cancellationToken.IsCancellationRequested && !pendingRequestsCts.IsCancellationRequested)
if (e is OperationCanceledException oce)
{
// If this exception is for cancellation, but cancellation wasn't requested, either by the caller's token or by the pending requests source,
// the only other cause could be a timeout. Treat it as such.
e = toThrow = new TaskCanceledException(SR.Format(SR.net_http_request_timedout, _timeout.TotalSeconds), new TimeoutException(e.Message, e), oce.CancellationToken);
if (cancellationToken.IsCancellationRequested)
{
if (oce.CancellationToken != cancellationToken)
{
// We got a cancellation exception, and the caller requested cancellation, but the exception doesn't contain that token.
// Massage things so that the cancellation exception we propagate appropriately contains the caller's token (it's possible
// multiple things caused cancellation, in which case we can attribute it to the caller's token, or it's possible the
// exception contains the linked token source, in which case that token isn't meaningful to the caller).
e = toThrow = new TaskCanceledException(oce.Message, oce.InnerException, cancellationToken);
}
}
else if (!pendingRequestsCts.IsCancellationRequested)
{
// If this exception is for cancellation, but cancellation wasn't requested, either by the caller's token or by the pending requests source,
// the only other cause could be a timeout. Treat it as such.
e = toThrow = new TaskCanceledException(SR.Format(SR.net_http_request_timedout, _timeout.TotalSeconds), new TimeoutException(e.Message, e), oce.CancellationToken);
}
}
else if (cts.IsCancellationRequested && e is HttpRequestException) // if cancellationToken is canceled, cts will also be canceled
else if (e is HttpRequestException && cts.IsCancellationRequested) // if cancellationToken is canceled, cts will also be canceled
{
// If the cancellation token source was canceled, race conditions abound, and we consider the failure to be
// caused by the cancellation (e.g. WebException when reading from canceled response stream).
e = toThrow = new OperationCanceledException(cts.Token);
e = toThrow = new OperationCanceledException(cancellationToken.IsCancellationRequested ? cancellationToken : cts.Token);
}

if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, e);
Expand Down
47 changes: 33 additions & 14 deletions src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,18 @@ public async Task GetPutPostDeleteAsync_Canceled_Throws()

cts.Cancel();

await Assert.ThrowsAsync<TaskCanceledException>(() => t1);
await Assert.ThrowsAsync<TaskCanceledException>(() => t2);
await Assert.ThrowsAsync<TaskCanceledException>(() => t3);
await Assert.ThrowsAsync<TaskCanceledException>(() => t4);
await Assert.ThrowsAsync<TaskCanceledException>(() => t5);
async Task ValidateCancellationAsync(Task t)
{
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => t);
Assert.Equal(cts.Token, tce.CancellationToken);

}

await ValidateCancellationAsync(t1);
await ValidateCancellationAsync(t2);
await ValidateCancellationAsync(t3);
await ValidateCancellationAsync(t4);
await ValidateCancellationAsync(t5);
}
}

Expand Down Expand Up @@ -382,7 +389,9 @@ await LoopbackServer.CreateClientAndServerAsync(
var cts = new CancellationTokenSource();
cts.Cancel();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStringAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStringAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
onClientFinished.Release();
},
async server =>
Expand All @@ -402,7 +411,8 @@ await LoopbackServer.CreateClientAndServerAsync(
{
using HttpClient httpClient = CreateHttpClient();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStringAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStringAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
},
async server =>
{
Expand Down Expand Up @@ -549,7 +559,9 @@ await LoopbackServer.CreateClientAndServerAsync(
var cts = new CancellationTokenSource();
cts.Cancel();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetByteArrayAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetByteArrayAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
onClientFinished.Release();
},
async server =>
Expand All @@ -569,7 +581,8 @@ await LoopbackServer.CreateClientAndServerAsync(
{
using HttpClient httpClient = CreateHttpClient();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetByteArrayAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetByteArrayAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
},
async server =>
{
Expand Down Expand Up @@ -623,7 +636,9 @@ await LoopbackServer.CreateClientAndServerAsync(
var cts = new CancellationTokenSource();
cts.Cancel();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStreamAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStreamAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
onClientFinished.Release();
},
async server =>
Expand All @@ -643,7 +658,8 @@ await LoopbackServer.CreateClientAndServerAsync(
{
using HttpClient httpClient = CreateHttpClient();
await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStreamAsync(uri, cts.Token));
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => httpClient.GetStreamAsync(uri, cts.Token));
Assert.Equal(cts.Token, tce.CancellationToken);
},
async server =>
{
Expand Down Expand Up @@ -729,7 +745,7 @@ public async Task Timeout_CallerCanceledTokenAfterTimeout_TimeoutIsNotDetected(H
cts.Cancel();
Task<HttpResponseMessage> task = client.GetAsync(CreateFakeUri(), completionOption, token);
OperationCanceledException e = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await task);
Assert.Null(e.InnerException);
Assert.Equal(e.CancellationToken, token);
}
}

Expand All @@ -746,7 +762,7 @@ public void Timeout_CallerCanceledTokenBeforeTimeout_TimeoutIsNotDetected(HttpCo
Task<HttpResponseMessage> task = client.GetAsync(CreateFakeUri(), completionOption, cts.Token);
cts.Cancel();
OperationCanceledException e = Assert.ThrowsAny<OperationCanceledException>(() => task.GetAwaiter().GetResult());
Assert.Null(e.InnerException);
Assert.Equal(e.CancellationToken, cts.Token);
}
}

Expand Down Expand Up @@ -821,7 +837,8 @@ public async Task PatchAsync_Canceled_Throws()

cts.Cancel();

await Assert.ThrowsAsync<TaskCanceledException>(() => t1);
TaskCanceledException tce = await Assert.ThrowsAsync<TaskCanceledException>(() => t1);
Assert.Equal(cts.Token, tce.CancellationToken);
}
}

Expand Down Expand Up @@ -966,6 +983,7 @@ await LoopbackServer.CreateClientAndServerAsync(
});
TaskCanceledException ex = await Assert.ThrowsAsync<TaskCanceledException>(() => sendTask);
Assert.Equal(cts.Token, ex.CancellationToken);
Assert.IsNotType<TimeoutException>(ex.InnerException);
},
async server =>
Expand Down Expand Up @@ -1050,6 +1068,7 @@ await LoopbackServer.CreateClientAndServerAsync(
});
TaskCanceledException ex = await Assert.ThrowsAsync<TaskCanceledException>(() => sendTask);
Assert.Equal(cts.Token, ex.CancellationToken);
Assert.IsNotType<TimeoutException>(ex.InnerException);
},
async server =>
Expand Down

0 comments on commit 42af85b

Please sign in to comment.