From: Alexander Nikolaev <55398552+alnikola@users.noreply.github.com>
Date: Wed, 19 Feb 2020 11:31:05 +0000 (+0100)
Subject: HttpClient throws TimeoutException wrapped by TaskCancellationException when request... 
X-Git-Tag: submit/tizen/20210909.063632~9632
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7818335aabed0e7acb843b0c4be1b86f807cbc3c;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

HttpClient throws TimeoutException wrapped by TaskCancellationException when request times out (#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 #21965
---

diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx
index 94beb1f..456e14c 100644
--- a/src/libraries/System.Net.Http/src/Resources/Strings.resx
+++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx
@@ -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>
\ No newline at end of file
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
index 05288da..ba716da 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
@@ -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;
 
@@ -491,12 +492,14 @@ namespace System.Net.Http
             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);
                 }
             }
@@ -512,19 +515,25 @@ namespace System.Net.Http
             {
                 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
@@ -548,6 +557,13 @@ namespace System.Net.Http
             catch (Exception e)
             {
                 response?.Dispose();
+
+                if (e is OperationCanceledException operationException && TimeoutFired(callerToken, timeoutTime))
+                {
+                    HandleSendAsyncTimeout(operationException);
+                    throw CreateTimeoutException(operationException);
+                }
+
                 HandleFinishSendAsyncError(e, cts);
                 throw;
             }
@@ -558,7 +574,7 @@ namespace System.Net.Http
         }
 
         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
             {
@@ -573,6 +589,12 @@ namespace System.Net.Http
             }
             catch (Exception e)
             {
+                if (e is OperationCanceledException operationException && TimeoutFired(callerToken, timeoutTime))
+                {
+                    HandleSendAsyncTimeout(operationException);
+                    throw CreateTimeoutException(operationException);
+                }
+
                 HandleFinishSendAsyncError(e, cts);
                 throw;
             }
@@ -582,6 +604,14 @@ namespace System.Net.Http
             }
         }
 
+        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);
@@ -595,6 +625,15 @@ namespace System.Net.Http
             }
         }
 
+        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
diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
index 83efb46..ad642fc 100644
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
@@ -593,14 +593,54 @@ namespace System.Net.Http.Functional.Tests
             }
         }
 
-        [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);
             }
         }