Handling HttpRequestException in CallHttpAsync() (#1718)

These changes add a way to handle when this.httpClient.SendAsync() throws an HttpRequestException and propagates the exception to customer code. Before this change, the extension would throw an exception about serialization instead of HttpRequestException. By handling the HttpRequestException in TaskHttpActivityShim, an HttpRequestException is thrown with a descriptive message.

Resolves #1669

Author: bachuv

release_notes.md

@@ -5,4 +5,7 @@
 ## Bug Fixes:
 - Properly used update management API URLs after a successful slot swap (#1716)
 - Fix race condition when multiple apps start with local RPC endpoints on the same VM in parallel. (#1719)
+- Fix CallHttpAsync() to throw an HttpRequestException instead of a serialization exception if the target endpoint doesn't exist (#1718)
 
+## Breaking changes
+- Fix CallHttpAsync() to throw an HttpRequestException instead of a serialization exception if the target endpoint doesn't exist (#1718). This is a breaking change if you were handling `HttpRequestException`s by catching `FunctionFailedException`s.

src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs

@@ -690,8 +690,14 @@ internal async Task<TResult> CallDurableTaskFunctionAsync<TResult>(
             {
                 // Check to see if CallHttpAsync() threw a TimeoutException
                 // In this case, we want to throw a TimeoutException instead of a FunctionFailedException
-                if (functionName.Equals(HttpOptions.HttpTaskActivityReservedName) && e.InnerException is TimeoutException)
+                if (functionName.Equals(HttpOptions.HttpTaskActivityReservedName) &&
+                    (e.InnerException is TimeoutException || e.InnerException is HttpRequestException))
                 {
+                    if (e.InnerException is HttpRequestException)
+                    {
+                        throw new HttpRequestException(e.Message);
+                    }
+
                     throw e.InnerException;
                 }
 

src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs

@@ -42,32 +42,40 @@ public async override Task<string> RunAsync(TaskContext context, string rawInput
             HttpRequestMessage requestMessage = await this.ConvertToHttpRequestMessage(durableHttpRequest);
 
             HttpResponseMessage response;
-            if (durableHttpRequest.Timeout == null)
+            try
             {
-                response = await this.httpClient.SendAsync(requestMessage);
-            }
-            else
-            {
-                try
+                if (durableHttpRequest.Timeout == null)
                 {
-                    using (CancellationTokenSource cts = new CancellationTokenSource())
-                    {
-                        cts.CancelAfter(durableHttpRequest.Timeout.Value);
-                        response = await this.httpClient.SendAsync(requestMessage, cts.Token);
-                    }
+                    response = await this.httpClient.SendAsync(requestMessage);
                 }
-                catch (OperationCanceledException ex)
+                else
                 {
-                    TimeoutException e = new TimeoutException(ex.Message + $" Reached user specified timeout: {durableHttpRequest.Timeout.Value}.");
+                    try
+                    {
+                        using (CancellationTokenSource cts = new CancellationTokenSource())
+                        {
+                            cts.CancelAfter(durableHttpRequest.Timeout.Value);
+                            response = await this.httpClient.SendAsync(requestMessage, cts.Token);
+                        }
+                    }
+                    catch (OperationCanceledException ex)
+                    {
+                        TimeoutException e = new TimeoutException(ex.Message + $" Reached user specified timeout: {durableHttpRequest.Timeout.Value}.");
 
-                    string details = Utils.SerializeCause(e, this.config.ErrorDataConverter);
-                    throw new TaskFailureException(e.Message, e, details);
+                        string details = Utils.SerializeCause(e, this.config.ErrorDataConverter);
+                        throw new TaskFailureException(e.Message, e, details);
+                    }
                 }
-            }
 
-            DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response);
+                DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response);
 
-            return JsonConvert.SerializeObject(durableHttpResponse);
+                return JsonConvert.SerializeObject(durableHttpResponse);
+            }
+            catch (HttpRequestException ex)
+            {
+                string details = Utils.SerializeCause(ex, this.config.ErrorDataConverter);
+                throw new TaskFailureException(ex.Message, ex, details);
+            }
         }
 
         private static DurableHttpRequest ReconstructDurableHttpRequest(string serializedRequest)

test/Common/DurableHttpTests.cs

@@ -362,6 +362,44 @@ public async Task DurableHttpAsync_Synchronous_TimeoutException(string storagePr
             }
         }
 
+        /// <summary>
+        /// End-to-end test which checks if the CallHttpAsync Orchestrator fails  when the
+        /// target url doesn't exist and throws an HttpRequestException.
+        /// </summary>
+        [Theory]
+        [Trait("Category", PlatformSpecificHelpers.TestCategory)]
+        [MemberData(nameof(TestDataGenerator.GetFullFeaturedStorageProviderOptions), MemberType = typeof(TestDataGenerator))]
+        public async Task DurableHttpAsync_Synchronous_HttpRequestException(string storageProvider)
+        {
+            HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithHttpRequestException();
+
+            using (ITestHost host = TestHelpers.GetJobHost(
+                this.loggerProvider,
+                nameof(this.DurableHttpAsync_Synchronous_HttpRequestException),
+                enableExtendedSessions: false,
+                storageProviderType: storageProvider,
+                durableHttpMessageHandler: new DurableHttpMessageHandlerFactory(httpMessageHandler)))
+            {
+                await host.StartAsync();
+
+                Dictionary<string, string> headers = new Dictionary<string, string>();
+                headers.Add("Accept", "application/json");
+                TestDurableHttpRequest testRequest = new TestDurableHttpRequest(
+                    httpMethod: HttpMethod.Get,
+                    headers: headers);
+
+                string functionName = nameof(TestOrchestrations.CallHttpAsyncOrchestrator);
+                var client = await host.StartOrchestratorAsync(functionName, testRequest, this.output);
+                var status = await client.WaitForCompletionAsync(this.output);
+
+                var output = status?.Output;
+                Assert.Contains("No such host is known.", output.ToString());
+                Assert.Equal(OrchestrationRuntimeStatus.Failed, status?.RuntimeStatus);
+
+                await host.StopAsync();
+            }
+        }
+
         /// <summary>
         /// End-to-end test which checks if the UserAgent header is set in the HttpResponseMessage.
         /// </summary>
@@ -1509,6 +1547,21 @@ private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeoutEx
             return handlerMock.Object;
         }
 
+        private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithHttpRequestException()
+        {
+            HttpResponseMessage httpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.NotFound);
+
+            httpResponseMessage.Content = new ExceptionThrowingContent(new HttpRequestException("No such host is known."));
+
+            var handlerMock = new Mock<HttpMessageHandler>(MockBehavior.Strict);
+            handlerMock
+               .Protected()
+               .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
+               .ReturnsAsync(httpResponseMessage);
+
+            return handlerMock.Object;
+        }
+
         private static HttpMessageHandler MockHttpMessageHandlerCheckUserAgent()
         {
             HttpResponseMessage okHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK);