From 1dbea163d4782897630a24275bbbb0c130609c82 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Wed, 4 Nov 2020 12:40:54 -0800 Subject: [PATCH 1/8] initial commit --- .../DurableOrchestrationContext.cs | 49 +++++-- .../DurableHttpRequest.cs | 19 ++- .../Listener/TaskHttpActivityShim.cs | 50 ++++++- ....WebJobs.Extensions.DurableTask-net461.xml | 17 ++- ...t.Azure.WebJobs.Extensions.DurableTask.xml | 17 ++- .../WebJobs.Extensions.DurableTask.csproj | 4 +- test/Common/DurableHttpTests.cs | 131 ++++++++++++++++++ test/Common/TestDurableHttpRequest.cs | 7 +- test/Common/TestOrchestrations.cs | 14 +- 9 files changed, 280 insertions(+), 28 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs index f87a5bfed..96d234588 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs @@ -243,7 +243,21 @@ Task IDurableOrchestrationContext.CallHttpAsync(HttpMethod async Task IDurableOrchestrationContext.CallHttpAsync(DurableHttpRequest req) { - DurableHttpResponse durableHttpResponse = await this.ScheduleDurableHttpActivityAsync(req); + // calculate timeout expiration if DurableHttpRequest.Timeout is set + if (req.Timeout != null) + { + req.TimeoutExpirationDateTime = this.InnerContext.CurrentUtcDateTime + req.Timeout.Value; + } + + DurableHttpResponse durableHttpResponse; + try + { + durableHttpResponse = await this.ScheduleDurableHttpActivityAsync(req); + } + catch (TimeoutException) + { + throw; + } HttpStatusCode currStatusCode = durableHttpResponse.StatusCode; @@ -266,7 +280,18 @@ async Task IDurableOrchestrationContext.CallHttpAsync(Durab } this.IncrementActionsOrThrowException(); - await this.InnerContext.CreateTimer(fireAt, CancellationToken.None); + + if (req.Timeout == null) + { + await this.InnerContext.CreateTimer(fireAt, CancellationToken.None); + } + else + { + TimeSpan timeLeft = req.TimeoutExpirationDateTime - this.InnerContext.CurrentUtcDateTime; + CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); + cancellationTokenSource.CancelAfter(timeLeft); + await this.InnerContext.CreateTimer(fireAt, cancellationTokenSource.Token); + } DurableHttpRequest durableAsyncHttpRequest = this.CreateLocationPollRequest( req, @@ -281,16 +306,16 @@ async Task IDurableOrchestrationContext.CallHttpAsync(Durab private async Task ScheduleDurableHttpActivityAsync(DurableHttpRequest req) { DurableHttpResponse durableHttpResponse = await this.CallDurableTaskFunctionAsync( - functionName: HttpOptions.HttpTaskActivityReservedName, - functionType: FunctionType.Activity, - oneWay: false, - instanceId: null, - operation: null, - retryOptions: null, - input: req, - scheduledTimeUtc: null); - - return durableHttpResponse; + functionName: HttpOptions.HttpTaskActivityReservedName, + functionType: FunctionType.Activity, + oneWay: false, + instanceId: null, + operation: null, + retryOptions: null, + input: req, + scheduledTimeUtc: null); + + return durableHttpResponse; } private DurableHttpRequest CreateLocationPollRequest(DurableHttpRequest durableHttpRequest, string locationUri) diff --git a/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs b/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs index 8effe0c53..b18c4695c 100644 --- a/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs +++ b/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs @@ -26,13 +26,15 @@ public class DurableHttpRequest /// Content added to the body of the HTTP request. /// AAD authentication attached to the HTTP request. /// Specifies whether the DurableHttpRequest should handle the asynchronous pattern. + /// TimeSpan used for HTTP request timeout. public DurableHttpRequest( HttpMethod method, Uri uri, IDictionary headers = null, string content = null, ITokenSource tokenSource = null, - bool asynchronousPatternEnabled = true) + bool asynchronousPatternEnabled = true, + TimeSpan? timeout = null) { this.Method = method; this.Uri = uri; @@ -40,6 +42,7 @@ public class DurableHttpRequest this.Content = content; this.TokenSource = tokenSource; this.AsynchronousPatternEnabled = asynchronousPatternEnabled; + this.Timeout = timeout; } /// @@ -82,6 +85,20 @@ public class DurableHttpRequest [JsonProperty("asynchronousPatternEnabled")] public bool AsynchronousPatternEnabled { get; } + /// + /// The total timeout for the original HTTP request and any + /// asynchronous polling. + /// + [JsonProperty("timeout")] + public TimeSpan? Timeout { get; } + + /// + /// The timeout expiration DateTime used to calculate when + /// the timeout will expire. + /// + [JsonProperty("timeoutExpiration")] + internal DateTime TimeoutExpirationDateTime { get; set; } + private class HttpMethodConverter : JsonConverter { public override bool CanConvert(Type objectType) diff --git a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs index 280f849ef..40f05ff17 100644 --- a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs +++ b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Net.Http.Headers; using System.Text; +using System.Threading; using System.Threading.Tasks; using DurableTask.Core; using Microsoft.Extensions.Primitives; @@ -35,20 +36,59 @@ public override string Run(TaskContext context, string input) public async override Task RunAsync(TaskContext context, string rawInput) { - HttpRequestMessage requestMessage = await this.ReconstructHttpRequestMessage(rawInput); - HttpResponseMessage response = await this.httpClient.SendAsync(requestMessage); - DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); + DurableHttpRequest durableHttpRequest = ReconstructDurableHttpRequest(rawInput); + HttpRequestMessage requestMessage = await this.ReconstructHttpRequestMessage(durableHttpRequest); - return JsonConvert.SerializeObject(durableHttpResponse); + CancellationTokenSource cts = new CancellationTokenSource(); + if (durableHttpRequest.Timeout != null) + { + try + { + TimeSpan timeout = durableHttpRequest.TimeoutExpirationDateTime - DateTime.UtcNow; + this.httpClient.Timeout = timeout; + cts.CancelAfter(timeout); + } + catch (Exception) + { + TimeSpan timeout = durableHttpRequest.Timeout.Value; + this.httpClient.Timeout = timeout; + cts.CancelAfter(timeout); + } + } + + try + { + HttpResponseMessage response; + if (durableHttpRequest.Timeout == null) + { + response = await this.httpClient.SendAsync(requestMessage); + } + else + { + response = await this.httpClient.SendAsync(requestMessage, cts.Token); + } + + DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); + + return JsonConvert.SerializeObject(durableHttpResponse); + } + catch (OperationCanceledException ex) // when (cts.Token.IsCancellationRequested) + { + throw new TimeoutException(ex.Message + $"Reached user specified timeout: {durableHttpRequest.Timeout.Value}."); + } } - private async Task ReconstructHttpRequestMessage(string serializedRequest) + private static DurableHttpRequest ReconstructDurableHttpRequest(string serializedRequest) { // DeserializeObject deserializes into a List and then the first element // of that list is the DurableHttpRequest IList input = JsonConvert.DeserializeObject>(serializedRequest); DurableHttpRequest durableHttpRequest = input.First(); + return durableHttpRequest; + } + private async Task ReconstructHttpRequestMessage(DurableHttpRequest durableHttpRequest) + { string contentType = ""; HttpRequestMessage requestMessage = new HttpRequestMessage(durableHttpRequest.Method, durableHttpRequest.Uri); if (durableHttpRequest.Headers != null) diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml index 0bcb5d515..afe2e1f5b 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml @@ -1781,7 +1781,7 @@ Initializes a new instance of the class. - durable client options + durable client options. @@ -1863,7 +1863,7 @@ Request used to make an HTTP call through Durable Functions. - + Initializes a new instance of the class. @@ -1873,6 +1873,7 @@ Content added to the body of the HTTP request. AAD authentication attached to the HTTP request. Specifies whether the DurableHttpRequest should handle the asynchronous pattern. + TimeSpan used for HTTP request. @@ -1905,6 +1906,18 @@ handle the asynchronous HTTP pattern. + + + The total timeout for the original HTTP request and any + asynchronous polling. + + + + + The timeout expiration DateTime used to calculate when + the timeout will expire. + + Response received from the HTTP request made by the Durable Function. diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml index c2a3102dd..20399bf1e 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml @@ -1786,7 +1786,7 @@ Initializes a new instance of the class. - durable client options + durable client options. @@ -1868,7 +1868,7 @@ Request used to make an HTTP call through Durable Functions. - + Initializes a new instance of the class. @@ -1878,6 +1878,7 @@ Content added to the body of the HTTP request. AAD authentication attached to the HTTP request. Specifies whether the DurableHttpRequest should handle the asynchronous pattern. + TimeSpan used for HTTP request. @@ -1910,6 +1911,18 @@ handle the asynchronous HTTP pattern. + + + The total timeout for the original HTTP request and any + asynchronous polling. + + + + + The timeout expiration DateTime used to calculate when + the timeout will expire. + + Response received from the HTTP request made by the Durable Function. diff --git a/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj b/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj index 189fafd7a..dcea7797f 100644 --- a/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj +++ b/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj @@ -6,7 +6,7 @@ Microsoft.Azure.WebJobs.Extensions.DurableTask 2 3 - 1 + 2 $(MajorVersion).$(MinorVersion).$(PatchVersion) $(MajorVersion).$(MinorVersion).$(PatchVersion) $(MajorVersion).0.0.0 @@ -77,7 +77,7 @@ - + diff --git a/test/Common/DurableHttpTests.cs b/test/Common/DurableHttpTests.cs index 578ea6f6f..0d444dd9a 100644 --- a/test/Common/DurableHttpTests.cs +++ b/test/Common/DurableHttpTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; @@ -310,6 +311,84 @@ public async Task DurableHttpAsync_SynchronousAPI_Returns200(string storageProvi } } + /// + /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code. + /// + [Theory] + [Trait("Category", PlatformSpecificHelpers.TestCategory)] + [MemberData(nameof(TestDataGenerator.GetFullFeaturedStorageProviderOptions), MemberType = typeof(TestDataGenerator))] + public async Task DurableHttpAsync_Synchronous_Timeout(string storageProvider) + { + HttpResponseMessage testHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); + HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeout(testHttpResponseMessage, TimeSpan.FromMilliseconds(10000)); + + using (ITestHost host = TestHelpers.GetJobHost( + this.loggerProvider, + nameof(this.DurableHttpAsync_Synchronous_Timeout), + enableExtendedSessions: false, + storageProviderType: storageProvider, + durableHttpMessageHandler: new DurableHttpMessageHandlerFactory(httpMessageHandler))) + { + await host.StartAsync(); + + Dictionary headers = new Dictionary(); + headers.Add("Accept", "application/json"); + TestDurableHttpRequest testRequest = new TestDurableHttpRequest( + httpMethod: HttpMethod.Get, + headers: headers, + timeout: TimeSpan.FromMilliseconds(5000)); + + string functionName = nameof(TestOrchestrations.CallHttpAsyncOrchestrator); + var client = await host.StartOrchestratorAsync(functionName, testRequest, this.output); + var status = await client.WaitForCompletionAsync(this.output, timeout: TimeSpan.FromSeconds(400)); + + var output = status?.Output; + DurableHttpResponse response = output.ToObject(); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + await host.StopAsync(); + } + } + + /// + /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code. + /// + [Theory] + [Trait("Category", PlatformSpecificHelpers.TestCategory)] + [MemberData(nameof(TestDataGenerator.GetFullFeaturedStorageProviderOptions), MemberType = typeof(TestDataGenerator))] + public async Task DurableHttpAsync_Synchronous_TimeoutException(string storageProvider) + { + HttpResponseMessage testHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); + HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeoutException(); + + using (ITestHost host = TestHelpers.GetJobHost( + this.loggerProvider, + nameof(this.DurableHttpAsync_Synchronous_TimeoutException), + enableExtendedSessions: false, + storageProviderType: storageProvider, + durableHttpMessageHandler: new DurableHttpMessageHandlerFactory(httpMessageHandler))) + { + await host.StartAsync(); + + Dictionary headers = new Dictionary(); + headers.Add("Accept", "application/json"); + TestDurableHttpRequest testRequest = new TestDurableHttpRequest( + httpMethod: HttpMethod.Get, + headers: headers, + timeout: TimeSpan.FromMilliseconds(5000)); + + string functionName = nameof(TestOrchestrations.CallHttpAsyncOrchestrator); + var client = await host.StartOrchestratorAsync(functionName, testRequest, this.output); + var status = await client.WaitForCompletionAsync(this.output, timeout: TimeSpan.FromSeconds(400)); + + var output = status?.Output; + Assert.Contains("System.TimeoutException: The operation was canceled.Reached user specified timeout", output.ToString()); + Assert.Equal(OrchestrationRuntimeStatus.Failed, status?.RuntimeStatus); + + await host.StopAsync(); + } + } + /// /// End-to-end test which checks if the UserAgent header is set in the HttpResponseMessage. /// @@ -1423,6 +1502,37 @@ private static HttpMessageHandler MockSynchronousHttpMessageHandler(HttpResponse return handlerMock.Object; } + private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeout(HttpResponseMessage httpResponseMessage, TimeSpan timeoutTimespan) + { + var handlerMock = new Mock(MockBehavior.Strict); + handlerMock + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(async () => + { + await Task.Delay(timeoutTimespan); + return httpResponseMessage; + }); + + return handlerMock.Object; + } + + private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeoutException() + { + HttpResponseMessage httpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); + CancellationTokenSource cts = new CancellationTokenSource(); + + httpResponseMessage.Content = new ExceptionThrowingContent(new OperationCanceledException()); + + var handlerMock = new Mock(MockBehavior.Strict); + handlerMock + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .ReturnsAsync(httpResponseMessage); + + return handlerMock.Object; + } + private static HttpMessageHandler MockHttpMessageHandlerCheckUserAgent() { HttpResponseMessage okHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); @@ -1612,5 +1722,26 @@ public ManagedIdentityOptions GetOptions() return this.options; } } + + private class ExceptionThrowingContent : HttpContent + { + private readonly Exception exception; + + public ExceptionThrowingContent(Exception exception) + { + this.exception = exception; + } + + protected override Task SerializeToStreamAsync(Stream stream, TransportContext context) + { + return Task.FromException(this.exception); + } + + protected override bool TryComputeLength(out long length) + { + length = 0L; + return false; + } + } } } diff --git a/test/Common/TestDurableHttpRequest.cs b/test/Common/TestDurableHttpRequest.cs index 7c4ebde73..a90346d34 100644 --- a/test/Common/TestDurableHttpRequest.cs +++ b/test/Common/TestDurableHttpRequest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. +using System; using System.Collections.Generic; using System.Net.Http; using System.Runtime.Serialization; @@ -17,13 +18,14 @@ namespace Microsoft.Azure.WebJobs.Extensions.DurableTask.Tests [DataContract] public class TestDurableHttpRequest { - public TestDurableHttpRequest(HttpMethod httpMethod, string uri = "https://www.dummy-url.com", IDictionary headers = null, string content = null, ITokenSource tokenSource = null) + public TestDurableHttpRequest(HttpMethod httpMethod, string uri = "https://www.dummy-url.com", IDictionary headers = null, string content = null, ITokenSource tokenSource = null, TimeSpan? timeout = null) { this.HttpMethod = httpMethod; this.Uri = uri; this.Headers = headers; this.Content = content; this.TokenSource = tokenSource; + this.Timeout = timeout; } [DataMember] @@ -46,5 +48,8 @@ public TestDurableHttpRequest(HttpMethod httpMethod, string uri = "https://www.d [DataMember] public bool AsynchronousPatternEnabled { get; set; } = true; + + [DataMember] + public TimeSpan? Timeout { get; set; } } } \ No newline at end of file diff --git a/test/Common/TestOrchestrations.cs b/test/Common/TestOrchestrations.cs index 1c484a536..3a8d6487c 100644 --- a/test/Common/TestOrchestrations.cs +++ b/test/Common/TestOrchestrations.cs @@ -498,8 +498,15 @@ public static async Task CallHttpAsyncOrchestrator([Orchest { TestDurableHttpRequest testRequest = ctx.GetInput(); DurableHttpRequest durableHttpRequest = ConvertTestRequestToDurableHttpRequest(testRequest); - DurableHttpResponse response = await ctx.CallHttpAsync(durableHttpRequest); - return response; + try + { + DurableHttpResponse response = await ctx.CallHttpAsync(durableHttpRequest); + return response; + } + catch (Exception e) + { + throw; + } } public static DurableHttpRequest ConvertTestRequestToDurableHttpRequest(TestDurableHttpRequest testRequest) @@ -529,7 +536,8 @@ public static DurableHttpRequest ConvertTestRequestToDurableHttpRequest(TestDura headers: testHeaders, content: testRequest.Content, tokenSource: testRequest.TokenSource, - asynchronousPatternEnabled: testRequest.AsynchronousPatternEnabled); + asynchronousPatternEnabled: testRequest.AsynchronousPatternEnabled, + timeout: testRequest.Timeout); return durableHttpRequest; } From 0dda1443383a2c6ed33edad9e72147e8b5e3f956 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Wed, 4 Nov 2020 16:18:58 -0800 Subject: [PATCH 2/8] revert package reference versions --- .../WebJobs.Extensions.DurableTask.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj b/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj index dcea7797f..189fafd7a 100644 --- a/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj +++ b/src/WebJobs.Extensions.DurableTask/WebJobs.Extensions.DurableTask.csproj @@ -6,7 +6,7 @@ Microsoft.Azure.WebJobs.Extensions.DurableTask 2 3 - 2 + 1 $(MajorVersion).$(MinorVersion).$(PatchVersion) $(MajorVersion).$(MinorVersion).$(PatchVersion) $(MajorVersion).0.0.0 @@ -77,7 +77,7 @@ - + From 14b38c0d161cf8cd7d5f15497ee308084f2ce870 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Wed, 4 Nov 2020 16:41:48 -0800 Subject: [PATCH 3/8] minor fixes --- .../DurableOrchestrationContext.cs | 20 +++++++++---------- ....WebJobs.Extensions.DurableTask-net461.xml | 4 ++-- ...t.Azure.WebJobs.Extensions.DurableTask.xml | 4 ++-- test/Common/DurableHttpTests.cs | 6 ++++-- test/Common/TestOrchestrations.cs | 2 +- 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs index 96d234588..08e837fdb 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs @@ -306,16 +306,16 @@ async Task IDurableOrchestrationContext.CallHttpAsync(Durab private async Task ScheduleDurableHttpActivityAsync(DurableHttpRequest req) { DurableHttpResponse durableHttpResponse = await this.CallDurableTaskFunctionAsync( - functionName: HttpOptions.HttpTaskActivityReservedName, - functionType: FunctionType.Activity, - oneWay: false, - instanceId: null, - operation: null, - retryOptions: null, - input: req, - scheduledTimeUtc: null); - - return durableHttpResponse; + functionName: HttpOptions.HttpTaskActivityReservedName, + functionType: FunctionType.Activity, + oneWay: false, + instanceId: null, + operation: null, + retryOptions: null, + input: req, + scheduledTimeUtc: null); + + return durableHttpResponse; } private DurableHttpRequest CreateLocationPollRequest(DurableHttpRequest durableHttpRequest, string locationUri) diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml index afe2e1f5b..e2f48008c 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml @@ -1781,7 +1781,7 @@ Initializes a new instance of the class. - durable client options. + durable client options @@ -1873,7 +1873,7 @@ Content added to the body of the HTTP request. AAD authentication attached to the HTTP request. Specifies whether the DurableHttpRequest should handle the asynchronous pattern. - TimeSpan used for HTTP request. + TimeSpan used for HTTP request timeout. diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml index 20399bf1e..2929dde7d 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml @@ -1786,7 +1786,7 @@ Initializes a new instance of the class. - durable client options. + durable client options @@ -1878,7 +1878,7 @@ Content added to the body of the HTTP request. AAD authentication attached to the HTTP request. Specifies whether the DurableHttpRequest should handle the asynchronous pattern. - TimeSpan used for HTTP request. + TimeSpan used for HTTP request timeout. diff --git a/test/Common/DurableHttpTests.cs b/test/Common/DurableHttpTests.cs index 0d444dd9a..e02ad27a1 100644 --- a/test/Common/DurableHttpTests.cs +++ b/test/Common/DurableHttpTests.cs @@ -312,7 +312,8 @@ public async Task DurableHttpAsync_SynchronousAPI_Returns200(string storageProvi } /// - /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code. + /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code + /// when a DurableHttpRequest timeout value is set. /// [Theory] [Trait("Category", PlatformSpecificHelpers.TestCategory)] @@ -351,7 +352,8 @@ public async Task DurableHttpAsync_Synchronous_Timeout(string storageProvider) } /// - /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code. + /// End-to-end test which checks if the CallHttpAsync Orchestrator fails when the + /// HTTP request times out and the CallHttpAsync API throws a TimeoutException. /// [Theory] [Trait("Category", PlatformSpecificHelpers.TestCategory)] diff --git a/test/Common/TestOrchestrations.cs b/test/Common/TestOrchestrations.cs index 3a8d6487c..c1a5b8b3d 100644 --- a/test/Common/TestOrchestrations.cs +++ b/test/Common/TestOrchestrations.cs @@ -503,7 +503,7 @@ public static async Task CallHttpAsyncOrchestrator([Orchest DurableHttpResponse response = await ctx.CallHttpAsync(durableHttpRequest); return response; } - catch (Exception e) + catch (Exception) { throw; } From 3638ac4c28751825abf07cbada2218b2c0e24ece Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Wed, 18 Nov 2020 10:46:33 -0800 Subject: [PATCH 4/8] addressed pr feedback --- .../DurableOrchestrationContext.cs | 32 ++----------- .../DurableHttpRequest.cs | 7 --- .../Listener/TaskHttpActivityShim.cs | 45 +++++++------------ ....WebJobs.Extensions.DurableTask-net461.xml | 6 --- ...t.Azure.WebJobs.Extensions.DurableTask.xml | 6 --- 5 files changed, 20 insertions(+), 76 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs index 08e837fdb..7988372b7 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs @@ -243,21 +243,7 @@ Task IDurableOrchestrationContext.CallHttpAsync(HttpMethod async Task IDurableOrchestrationContext.CallHttpAsync(DurableHttpRequest req) { - // calculate timeout expiration if DurableHttpRequest.Timeout is set - if (req.Timeout != null) - { - req.TimeoutExpirationDateTime = this.InnerContext.CurrentUtcDateTime + req.Timeout.Value; - } - - DurableHttpResponse durableHttpResponse; - try - { - durableHttpResponse = await this.ScheduleDurableHttpActivityAsync(req); - } - catch (TimeoutException) - { - throw; - } + DurableHttpResponse durableHttpResponse = await this.ScheduleDurableHttpActivityAsync(req); HttpStatusCode currStatusCode = durableHttpResponse.StatusCode; @@ -280,18 +266,7 @@ async Task IDurableOrchestrationContext.CallHttpAsync(Durab } this.IncrementActionsOrThrowException(); - - if (req.Timeout == null) - { - await this.InnerContext.CreateTimer(fireAt, CancellationToken.None); - } - else - { - TimeSpan timeLeft = req.TimeoutExpirationDateTime - this.InnerContext.CurrentUtcDateTime; - CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); - cancellationTokenSource.CancelAfter(timeLeft); - await this.InnerContext.CreateTimer(fireAt, cancellationTokenSource.Token); - } + await this.InnerContext.CreateTimer(fireAt, CancellationToken.None); DurableHttpRequest durableAsyncHttpRequest = this.CreateLocationPollRequest( req, @@ -324,7 +299,8 @@ private DurableHttpRequest CreateLocationPollRequest(DurableHttpRequest durableH method: HttpMethod.Get, uri: new Uri(locationUri), headers: durableHttpRequest.Headers, - tokenSource: durableHttpRequest.TokenSource); + tokenSource: durableHttpRequest.TokenSource, + timeout: durableHttpRequest.Timeout); // Do not copy over the x-functions-key header, as in many cases, the // functions key used for the initial request will be a Function-level key diff --git a/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs b/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs index b18c4695c..a4cdf41a2 100644 --- a/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs +++ b/src/WebJobs.Extensions.DurableTask/DurableHttpRequest.cs @@ -92,13 +92,6 @@ public class DurableHttpRequest [JsonProperty("timeout")] public TimeSpan? Timeout { get; } - /// - /// The timeout expiration DateTime used to calculate when - /// the timeout will expire. - /// - [JsonProperty("timeoutExpiration")] - internal DateTime TimeoutExpirationDateTime { get; set; } - private class HttpMethodConverter : JsonConverter { public override bool CanConvert(Type objectType) diff --git a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs index 40f05ff17..8532e8c60 100644 --- a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs +++ b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs @@ -39,42 +39,29 @@ public async override Task RunAsync(TaskContext context, string rawInput DurableHttpRequest durableHttpRequest = ReconstructDurableHttpRequest(rawInput); HttpRequestMessage requestMessage = await this.ReconstructHttpRequestMessage(durableHttpRequest); - CancellationTokenSource cts = new CancellationTokenSource(); - if (durableHttpRequest.Timeout != null) + using (CancellationTokenSource cts = new CancellationTokenSource()) { try { - TimeSpan timeout = durableHttpRequest.TimeoutExpirationDateTime - DateTime.UtcNow; - this.httpClient.Timeout = timeout; - cts.CancelAfter(timeout); - } - catch (Exception) - { - TimeSpan timeout = durableHttpRequest.Timeout.Value; - this.httpClient.Timeout = timeout; - cts.CancelAfter(timeout); - } - } + HttpResponseMessage response; + if (durableHttpRequest.Timeout == null) + { + response = await this.httpClient.SendAsync(requestMessage); + } + else + { + cts.CancelAfter(durableHttpRequest.Timeout.Value); + response = await this.httpClient.SendAsync(requestMessage, cts.Token); + } - try - { - HttpResponseMessage response; - if (durableHttpRequest.Timeout == null) - { - response = await this.httpClient.SendAsync(requestMessage); + DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); + + return JsonConvert.SerializeObject(durableHttpResponse); } - else + catch (OperationCanceledException ex) // when (cts.Token.IsCancellationRequested) { - response = await this.httpClient.SendAsync(requestMessage, cts.Token); + throw new TimeoutException(ex.Message + $"Reached user specified timeout: {durableHttpRequest.Timeout.Value}."); } - - DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); - - return JsonConvert.SerializeObject(durableHttpResponse); - } - catch (OperationCanceledException ex) // when (cts.Token.IsCancellationRequested) - { - throw new TimeoutException(ex.Message + $"Reached user specified timeout: {durableHttpRequest.Timeout.Value}."); } } diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml index e2f48008c..7f97533f1 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml @@ -1912,12 +1912,6 @@ asynchronous polling. - - - The timeout expiration DateTime used to calculate when - the timeout will expire. - - Response received from the HTTP request made by the Durable Function. diff --git a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml index 2929dde7d..320695bb0 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml @@ -1917,12 +1917,6 @@ asynchronous polling. - - - The timeout expiration DateTime used to calculate when - the timeout will expire. - - Response received from the HTTP request made by the Durable Function. From c7cc876954a8a7f66357d6c6d9135cceed128ed7 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Thu, 19 Nov 2020 17:15:55 -0800 Subject: [PATCH 5/8] added fix to throw a TimeoutException instead of FunctionFailedException. --- .../ContextImplementations/DurableOrchestrationContext.cs | 7 +++++++ .../Listener/TaskHttpActivityShim.cs | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs index 7988372b7..353ed95c1 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableOrchestrationContext.cs @@ -694,6 +694,13 @@ string IDurableOrchestrationContext.StartNewOrchestration(string functionName, o } catch (TaskFailedException e) { + // 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) + { + throw e.InnerException; + } + exception = e; string message = string.Format( "The {0} function '{1}' failed: \"{2}\". See the function execution logs for additional details.", diff --git a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs index 8532e8c60..727f57cf9 100644 --- a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs +++ b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs @@ -10,6 +10,8 @@ using System.Threading; using System.Threading.Tasks; using DurableTask.Core; +using DurableTask.Core.Common; +using DurableTask.Core.Exceptions; using Microsoft.Extensions.Primitives; using Newtonsoft.Json; @@ -60,7 +62,10 @@ public async override Task RunAsync(TaskContext context, string rawInput } catch (OperationCanceledException ex) // when (cts.Token.IsCancellationRequested) { - throw new TimeoutException(ex.Message + $"Reached user specified timeout: {durableHttpRequest.Timeout.Value}."); + 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); } } } From 875772b41ed93ddb67f4e076c7831651062231a9 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Thu, 19 Nov 2020 17:59:08 -0800 Subject: [PATCH 6/8] updated tests --- test/Common/DurableHttpTests.cs | 21 ++++++++++++--------- test/Common/TestOrchestrations.cs | 11 ++--------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/test/Common/DurableHttpTests.cs b/test/Common/DurableHttpTests.cs index e02ad27a1..c80f7e297 100644 --- a/test/Common/DurableHttpTests.cs +++ b/test/Common/DurableHttpTests.cs @@ -313,19 +313,19 @@ public async Task DurableHttpAsync_SynchronousAPI_Returns200(string storageProvi /// /// End-to-end test which checks if the CallHttpAsync Orchestrator returns an OK (200) status code - /// when a DurableHttpRequest timeout value is set. + /// when a DurableHttpRequest timeout value is set and the request completes within the timeout. /// [Theory] [Trait("Category", PlatformSpecificHelpers.TestCategory)] [MemberData(nameof(TestDataGenerator.GetFullFeaturedStorageProviderOptions), MemberType = typeof(TestDataGenerator))] - public async Task DurableHttpAsync_Synchronous_Timeout(string storageProvider) + public async Task DurableHttpAsync_Synchronous_TimeoutNotReached(string storageProvider) { HttpResponseMessage testHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); - HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeout(testHttpResponseMessage, TimeSpan.FromMilliseconds(10000)); + HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeout(testHttpResponseMessage, TimeSpan.FromMilliseconds(2000)); using (ITestHost host = TestHelpers.GetJobHost( this.loggerProvider, - nameof(this.DurableHttpAsync_Synchronous_Timeout), + nameof(this.DurableHttpAsync_Synchronous_TimeoutNotReached), enableExtendedSessions: false, storageProviderType: storageProvider, durableHttpMessageHandler: new DurableHttpMessageHandlerFactory(httpMessageHandler))) @@ -361,7 +361,7 @@ public async Task DurableHttpAsync_Synchronous_Timeout(string storageProvider) public async Task DurableHttpAsync_Synchronous_TimeoutException(string storageProvider) { HttpResponseMessage testHttpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); - HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeoutException(); + HttpMessageHandler httpMessageHandler = MockSynchronousHttpMessageHandlerWithTimeoutException(TimeSpan.FromMilliseconds(10000)); using (ITestHost host = TestHelpers.GetJobHost( this.loggerProvider, @@ -384,7 +384,7 @@ public async Task DurableHttpAsync_Synchronous_TimeoutException(string storagePr var status = await client.WaitForCompletionAsync(this.output, timeout: TimeSpan.FromSeconds(400)); var output = status?.Output; - Assert.Contains("System.TimeoutException: The operation was canceled.Reached user specified timeout", output.ToString()); + Assert.Contains("Orchestrator function 'CallHttpAsyncOrchestrator' failed: The operation was canceled. Reached user specified timeout: 00:00:05", output.ToString()); Assert.Equal(OrchestrationRuntimeStatus.Failed, status?.RuntimeStatus); await host.StopAsync(); @@ -1519,10 +1519,9 @@ private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeout(H return handlerMock.Object; } - private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeoutException() + private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeoutException(TimeSpan timeoutTimespan) { HttpResponseMessage httpResponseMessage = CreateTestHttpResponseMessage(HttpStatusCode.OK); - CancellationTokenSource cts = new CancellationTokenSource(); httpResponseMessage.Content = new ExceptionThrowingContent(new OperationCanceledException()); @@ -1530,7 +1529,11 @@ private static HttpMessageHandler MockSynchronousHttpMessageHandlerWithTimeoutEx handlerMock .Protected() .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .ReturnsAsync(httpResponseMessage); + .Returns(async () => + { + await Task.Delay(timeoutTimespan); + return httpResponseMessage; + }); return handlerMock.Object; } diff --git a/test/Common/TestOrchestrations.cs b/test/Common/TestOrchestrations.cs index c1a5b8b3d..13acba38b 100644 --- a/test/Common/TestOrchestrations.cs +++ b/test/Common/TestOrchestrations.cs @@ -498,15 +498,8 @@ public static async Task CallHttpAsyncOrchestrator([Orchest { TestDurableHttpRequest testRequest = ctx.GetInput(); DurableHttpRequest durableHttpRequest = ConvertTestRequestToDurableHttpRequest(testRequest); - try - { - DurableHttpResponse response = await ctx.CallHttpAsync(durableHttpRequest); - return response; - } - catch (Exception) - { - throw; - } + DurableHttpResponse response = await ctx.CallHttpAsync(durableHttpRequest); + return response; } public static DurableHttpRequest ConvertTestRequestToDurableHttpRequest(TestDurableHttpRequest testRequest) From 62353e7e07cd9c15a284b466e071bd2b4c1bd2f9 Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Mon, 23 Nov 2020 10:41:41 -0800 Subject: [PATCH 7/8] made minor fixes to TaskHttpActivityShim --- .../Listener/TaskHttpActivityShim.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs index 727f57cf9..02a7b59cc 100644 --- a/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs +++ b/src/WebJobs.Extensions.DurableTask/Listener/TaskHttpActivityShim.cs @@ -39,28 +39,24 @@ public override string Run(TaskContext context, string input) public async override Task RunAsync(TaskContext context, string rawInput) { DurableHttpRequest durableHttpRequest = ReconstructDurableHttpRequest(rawInput); - HttpRequestMessage requestMessage = await this.ReconstructHttpRequestMessage(durableHttpRequest); + HttpRequestMessage requestMessage = await this.ConvertToHttpRequestMessage(durableHttpRequest); - using (CancellationTokenSource cts = new CancellationTokenSource()) + HttpResponseMessage response; + if (durableHttpRequest.Timeout == null) + { + response = await this.httpClient.SendAsync(requestMessage); + } + else { try { - HttpResponseMessage response; - if (durableHttpRequest.Timeout == null) - { - response = await this.httpClient.SendAsync(requestMessage); - } - else + using (CancellationTokenSource cts = new CancellationTokenSource()) { cts.CancelAfter(durableHttpRequest.Timeout.Value); response = await this.httpClient.SendAsync(requestMessage, cts.Token); } - - DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); - - return JsonConvert.SerializeObject(durableHttpResponse); } - catch (OperationCanceledException ex) // when (cts.Token.IsCancellationRequested) + catch (OperationCanceledException ex) { TimeoutException e = new TimeoutException(ex.Message + $" Reached user specified timeout: {durableHttpRequest.Timeout.Value}."); @@ -68,6 +64,10 @@ public async override Task RunAsync(TaskContext context, string rawInput throw new TaskFailureException(e.Message, e, details); } } + + DurableHttpResponse durableHttpResponse = await DurableHttpResponse.CreateDurableHttpResponseWithHttpResponseMessage(response); + + return JsonConvert.SerializeObject(durableHttpResponse); } private static DurableHttpRequest ReconstructDurableHttpRequest(string serializedRequest) @@ -79,7 +79,7 @@ private static DurableHttpRequest ReconstructDurableHttpRequest(string serialize return durableHttpRequest; } - private async Task ReconstructHttpRequestMessage(DurableHttpRequest durableHttpRequest) + private async Task ConvertToHttpRequestMessage(DurableHttpRequest durableHttpRequest) { string contentType = ""; HttpRequestMessage requestMessage = new HttpRequestMessage(durableHttpRequest.Method, durableHttpRequest.Uri); From 1f5e04dd7dc24dfddead2b0c997498a15846ea6d Mon Sep 17 00:00:00 2001 From: Varshi Bachu Date: Mon, 23 Nov 2020 11:42:51 -0800 Subject: [PATCH 8/8] updated Durable HTTP serialization tests to include the new timeout property --- test/Common/DurableHttpTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/Common/DurableHttpTests.cs b/test/Common/DurableHttpTests.cs index c80f7e297..13894d975 100644 --- a/test/Common/DurableHttpTests.cs +++ b/test/Common/DurableHttpTests.cs @@ -145,7 +145,8 @@ public void SerializeManagedIdentityOptions() ""tenantid"": ""tenant_id"" } }, - ""AsynchronousPatternEnabled"": true + ""AsynchronousPatternEnabled"": true, + ""Timeout"": null }"; Dictionary headers = new Dictionary(); @@ -183,7 +184,8 @@ public void SerializeManagedIdentityOptions() ""tenantid"": ""tenant_id"" } }, - ""asynchronousPatternEnabled"": true + ""asynchronousPatternEnabled"": true, + ""timeout"": null }"; ManagedIdentityTokenSource managedIdentityTokenSource = new ManagedIdentityTokenSource("dummy url", options); TestDurableHttpRequest testDurableHttpRequest = new TestDurableHttpRequest( @@ -213,7 +215,8 @@ public void SerializeDurableHttpRequestWithoutManagedIdentityOptions() ""kind"": ""AzureManagedIdentity"", ""resource"": ""dummy url"" }, - ""asynchronousPatternEnabled"": true + ""asynchronousPatternEnabled"": true, + ""timeout"": null }"; Dictionary headers = new Dictionary();