Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the token included by HttpClient.HandleFailure #53133

Merged
merged 1 commit into from
May 24, 2021
Merged

Fix the token included by HttpClient.HandleFailure #53133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
View file
Open in desktop
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
Open in desktop
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);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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