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.

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

Unexpected TaskCanceledException when HttpClient.SendAsync fails possibly due to connection issue #35340

Closed
appel1 opened this issue Apr 23, 2020 · 13 comments
Assignees
Labels
area-System.Net.Http documentation Documentation bug or enhancement, does not impact product or test code
Milestone

Comments

@appel1
Copy link

appel1 commented Apr 23, 2020

.NET Core 3.1

Is it intentional that HttpClient.SendAsync throws TaskCanceledException when the connection is aborted? It doesn't work very well together with DataFlow that silently swallows TaskCanceledException.

Is this behavior documented anywhere so we can rely on the fact that a TaskCanceledException with an inner IOException means that we had some kind of connection failure and is not a cancelation at all?

System.Threading.Tasks.TaskCanceledException: The operation was canceled.
 ---> System.IO.IOException: The response ended prematurely.
   at System.Net.Http.HttpConnection.FillAsync()
   at System.Net.Http.HttpConnection.ReadNextResponseHeaderLineAsync(Boolean foldedHeadersAllowed)
   at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithNtConnectionAuthAsync(HttpConnection connection, HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.FinishSendAsyncUnbuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
   at HttpClientWebConnectorRequestor.SendRequestAsync(Uri requestUri, String sessionId, XNode postXml, String acceptedContentType, SendRequestOptions options, Nullable`1 lastModified, HashSet`1 acceptedCertificateThumbnails, CancellationToken cancellationToken) in HttpClientWebConnectorRequestor.cs:line 76
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Apr 23, 2020
@ghost
Copy link

ghost commented Apr 23, 2020

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

@davidsh
Copy link
Contributor

davidsh commented Apr 23, 2020

@alnikola

@wfurt
Copy link
Member

wfurt commented Apr 23, 2020

You can get cancel exception on timeout @appel1. It is possible timeout hit when waiting in FillAsyns. You can play with Timeout value on HttpClientHandler as well as do packet capture.

@thriolsson
Copy link

Seems to me like you would like TaskCanceledException to be reserved for the case when a task is canceled using a CancellationToken. This would be my expectation.

@appel1
Copy link
Author

appel1 commented Apr 23, 2020

@wfurt That's very unexpected considering the documentation states that SendAsync throws a HttpRequestException for timeouts.

@karelz karelz changed the title .NET Core 3.1 Unexpected TaskCanceledException when HttpClient.SendAsync fails possibly due to connection issue Unexpected TaskCanceledException when HttpClient.SendAsync fails possibly due to connection issue Apr 23, 2020
@wfurt
Copy link
Member

wfurt commented Apr 24, 2020

Do you have repro @appel1 ? You can read more about this here #21965 and it clearly looks like a duplicate.
You can also give it try with 5.0 preview builds.

@wfurt
Copy link
Member

wfurt commented Apr 24, 2020

I'm wondering @karelz if we could update the docs since we clearly have versions with different behavior. Perhaps simply remove references to timeout.

@appel1
Copy link
Author

appel1 commented Apr 24, 2020

@wfurt Yes, it looks like the same as that issue. And depending on where you are in SendAsync you get different inner exceptions or none in the TaskCanceledException.

Don't agree that using TaskCanceledException in the case of a timeout is a good idea. Very unintuitive and breaks things like DataFlow that doesn't expect you to misuse TaskCanceledException in this manner.

@wfurt
Copy link
Member

wfurt commented Apr 24, 2020

give it try with 5.0. It is unlikely 3.x will change at this point.

@appel1
Copy link
Author

appel1 commented Apr 24, 2020

Looks like 5.0 still throws a TaskCancelledException for timeouts, but a little nicer since the mandatory try-catch can now inspect the inner exception to find out that it was a timeout.

Hopefully this can be highlighted very clearly in the docs since it causes a mess if you are not careful together with DataFlow.

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

@karelz karelz added documentation Documentation bug or enhancement, does not impact product or test code and removed untriaged New issue has not been triaged by the area owner labels May 26, 2020
@karelz karelz added this to the 5.0 milestone May 26, 2020
@karelz
Copy link
Member

karelz commented May 26, 2020

Triage:

  • Addressed in 5.0 in HttpClient throws TaskCanceledException on timeout #21965
  • We need to fix the docs -- address specific versions (.NET Core different versions vs. .NET Framework) ... Note: there is only 1 page for all versions
    • Take into account that HttpRequestException may have been used in past versions (.NET Framework)

@alnikola
Copy link
Contributor

alnikola commented Jun 5, 2020

Created a PR in docs repo dotnet/dotnet-api-docs#4340

@alnikola
Copy link
Contributor

Documentation PR dotnet/dotnet-api-docs#4340 is merged.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http documentation Documentation bug or enhancement, does not impact product or test code
Projects
None yet
Development

No branches or pull requests

7 participants