From c7e8da1b9bbb08ec0210ea6513a8970337d58d99 Mon Sep 17 00:00:00 2001 From: amdeel <52223332+amdeel@users.noreply.github.com> Date: Wed, 18 Nov 2020 15:36:53 -0800 Subject: [PATCH 1/4] added returnInternalServerErrorOnFailure parameter --- .../ContextImplementations/DurableClient.cs | 13 +++--- .../IDurableOrchestrationClient.cs | 12 +++++- .../HttpApiHandler.cs | 5 ++- ....WebJobs.Extensions.DurableTask-net461.xml | 16 ++++--- ...t.Azure.WebJobs.Extensions.DurableTask.xml | 16 ++++--- test/Common/HttpApiHandlerTests.cs | 42 ++++++++++++++++++- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs index bd9f47498..5fc10be37 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs @@ -104,18 +104,19 @@ HttpManagementPayload IDurableOrchestrationClient.CreateHttpManagementPayload(st } /// - async Task IDurableOrchestrationClient.WaitForCompletionOrCreateCheckStatusResponseAsync(HttpRequestMessage request, string instanceId, TimeSpan? timeout, TimeSpan? retryInterval) + async Task IDurableOrchestrationClient.WaitForCompletionOrCreateCheckStatusResponseAsync(HttpRequestMessage request, string instanceId, TimeSpan? timeout, TimeSpan? retryInterval, bool returnInternalServerErrorOnFailure) { return await this.WaitForCompletionOrCreateCheckStatusResponseAsync( request, instanceId, this.attribute, timeout ?? TimeSpan.FromSeconds(10), - retryInterval ?? TimeSpan.FromSeconds(1)); + retryInterval ?? TimeSpan.FromSeconds(1), + returnInternalServerErrorOnFailure); } /// - async Task IDurableOrchestrationClient.WaitForCompletionOrCreateCheckStatusResponseAsync(HttpRequest request, string instanceId, TimeSpan? timeout, TimeSpan? retryInterval) + async Task IDurableOrchestrationClient.WaitForCompletionOrCreateCheckStatusResponseAsync(HttpRequest request, string instanceId, TimeSpan? timeout, TimeSpan? retryInterval, bool returnInternalServerErrorOnFailure) { HttpRequestMessage requestMessage = ConvertHttpRequestMessage(request); HttpResponseMessage responseMessage = await ((IDurableOrchestrationClient)this).WaitForCompletionOrCreateCheckStatusResponseAsync(requestMessage, instanceId, timeout, retryInterval); @@ -661,14 +662,16 @@ internal async Task WaitForCompletionOrCreateCheckStatusRes string instanceId, DurableClientAttribute attribute, TimeSpan timeout, - TimeSpan retryInterval) + TimeSpan retryInterval, + bool returnInternalServerErrorOnFailure) { return await this.httpApiHandler.WaitForCompletionOrCreateCheckStatusResponseAsync( request, instanceId, attribute, timeout, - retryInterval); + retryInterval, + returnInternalServerErrorOnFailure); } private static bool IsOrchestrationRunning(OrchestrationState status) diff --git a/src/WebJobs.Extensions.DurableTask/ContextInterfaces/IDurableOrchestrationClient.cs b/src/WebJobs.Extensions.DurableTask/ContextInterfaces/IDurableOrchestrationClient.cs index 52a35207c..5055bfe07 100644 --- a/src/WebJobs.Extensions.DurableTask/ContextInterfaces/IDurableOrchestrationClient.cs +++ b/src/WebJobs.Extensions.DurableTask/ContextInterfaces/IDurableOrchestrationClient.cs @@ -78,12 +78,16 @@ public interface IDurableOrchestrationClient /// The unique ID of the instance to check. /// Total allowed timeout for output from the durable function. The default value is 10 seconds. /// The timeout between checks for output from the durable function. The default value is 1 second. + /// Optional parameter that configures the http response code returned. Defaults to false. + /// If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + /// return 200. /// An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. Task WaitForCompletionOrCreateCheckStatusResponseAsync( HttpRequestMessage request, string instanceId, TimeSpan? timeout = null, - TimeSpan? retryInterval = null); + TimeSpan? retryInterval = null, + bool returnInternalServerErrorOnFailure = false); /// /// Creates an HTTP response which either contains a payload of management URLs for a non-completed instance @@ -99,12 +103,16 @@ Task WaitForCompletionOrCreateCheckStatusResponseAsync( /// The unique ID of the instance to check. /// Total allowed timeout for output from the durable function. The default value is 10 seconds. /// The timeout between checks for output from the durable function. The default value is 1 second. + /// Optional parameter that configures the http response code returned. Defaults to false. + /// If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + /// return 200. /// An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. Task WaitForCompletionOrCreateCheckStatusResponseAsync( HttpRequest request, string instanceId, TimeSpan? timeout = null, - TimeSpan? retryInterval = null); + TimeSpan? retryInterval = null, + bool returnInternalServerErrorOnFailure = false); /// /// Starts a new execution of the specified orchestrator function. diff --git a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs index 891b46f6e..a1ca05e32 100644 --- a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs +++ b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs @@ -170,14 +170,15 @@ internal async Task WaitForCompletionOrCreateCheckStatusRes string instanceId, DurableClientAttribute attribute, TimeSpan timeout, - TimeSpan retryInterval) + TimeSpan retryInterval, + bool returnInternalServerErrorOnFailure = false) { if (retryInterval > timeout) { throw new ArgumentException($"Total timeout {timeout.TotalSeconds} should be bigger than retry timeout {retryInterval.TotalSeconds}"); } - HttpManagementPayload httpManagementPayload = this.GetClientResponseLinks(request, instanceId, attribute?.TaskHub, attribute?.ConnectionName); + HttpManagementPayload httpManagementPayload = this.GetClientResponseLinks(request, instanceId, attribute?.TaskHub, attribute?.ConnectionName, returnInternalServerErrorOnFailure); IDurableOrchestrationClient client = this.GetClient(request); Stopwatch stopwatch = Stopwatch.StartNew(); 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..8d0a2074b 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 @@ -108,10 +108,10 @@ - + - + @@ -744,7 +744,7 @@ The ID of the orchestration instance to check. Instance of the class. - + Creates an HTTP response which either contains a payload of management URLs for a non-completed instance or contains the payload containing the output of the completed orchestration. @@ -759,9 +759,12 @@ The unique ID of the instance to check. Total allowed timeout for output from the durable function. The default value is 10 seconds. The timeout between checks for output from the durable function. The default value is 1 second. + Optional parameter that configures the http response code returned. Defaults to false. + If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + return 200. An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. - + Creates an HTTP response which either contains a payload of management URLs for a non-completed instance or contains the payload containing the output of the completed orchestration. @@ -776,6 +779,9 @@ The unique ID of the instance to check. Total allowed timeout for output from the durable function. The default value is 10 seconds. The timeout between checks for output from the durable function. The default value is 1 second. + Optional parameter that configures the http response code returned. Defaults to false. + If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + return 200. An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. @@ -1781,7 +1787,7 @@ Initializes a new instance of the class. - durable client options + Options to configure the IDurableClient created. 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..24898bf71 100644 --- a/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml +++ b/src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml @@ -108,10 +108,10 @@ - + - + @@ -749,7 +749,7 @@ The ID of the orchestration instance to check. Instance of the class. - + Creates an HTTP response which either contains a payload of management URLs for a non-completed instance or contains the payload containing the output of the completed orchestration. @@ -764,9 +764,12 @@ The unique ID of the instance to check. Total allowed timeout for output from the durable function. The default value is 10 seconds. The timeout between checks for output from the durable function. The default value is 1 second. + Optional parameter that configures the http response code returned. Defaults to false. + If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + return 200. An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. - + Creates an HTTP response which either contains a payload of management URLs for a non-completed instance or contains the payload containing the output of the completed orchestration. @@ -781,6 +784,9 @@ The unique ID of the instance to check. Total allowed timeout for output from the durable function. The default value is 10 seconds. The timeout between checks for output from the durable function. The default value is 1 second. + Optional parameter that configures the http response code returned. Defaults to false. + If true, the returned http response code will be a 500 when the orchestrator is in a failed state, when false it will + return 200. An HTTP response which may include a 202 and location header or a 200 with the durable function output in the response body. @@ -1786,7 +1792,7 @@ Initializes a new instance of the class. - durable client options + Options to configure the IDurableClient created. diff --git a/test/Common/HttpApiHandlerTests.cs b/test/Common/HttpApiHandlerTests.cs index d01c9d575..ec16ccfc6 100644 --- a/test/Common/HttpApiHandlerTests.cs +++ b/test/Common/HttpApiHandlerTests.cs @@ -185,6 +185,46 @@ public void CreateCheckStatus_Returns_Correct_HttpManagementPayload_based_on_cus httpManagementPayload.PurgeHistoryDeleteUri); } + [Fact] + [Trait("Category", PlatformSpecificHelpers.TestCategory)] + public async Task WaitForCompletionOrCreateCheckStatusResponseAsync_Returns_Custom_HttpManagementPayload_After_Timeout() + { + var httpApiHandler = new HttpApiHandler(GetTestExtension(), null); + var stopWatch = Stopwatch.StartNew(); + var httpResponseMessage = await httpApiHandler.WaitForCompletionOrCreateCheckStatusResponseAsync( + new HttpRequestMessage + { + RequestUri = new Uri(TestConstants.RequestUri), + }, + TestConstants.RandomInstanceId, + new DurableClientAttribute + { + TaskHub = TestConstants.TaskHub, + ConnectionName = TestConstants.ConnectionName, + }, + TimeSpan.FromSeconds(100), + TimeSpan.FromSeconds(10), + true); + stopWatch.Stop(); + Assert.Equal(HttpStatusCode.Accepted, httpResponseMessage.StatusCode); + var content = await httpResponseMessage.Content.ReadAsStringAsync(); + var status = JsonConvert.DeserializeObject(content); + Assert.Equal(status["id"], TestConstants.RandomInstanceId); + Assert.Equal( + $"{TestConstants.NotificationUrlBase}/instances/9b59154ae666471993659902ed0ba749?taskHub=SampleHubVS&connection=Storage&code=mykey&returnInternalServerErrorOnFailure=true", + (string)status["statusQueryGetUri"]); + Assert.Equal( + $"{TestConstants.NotificationUrlBase}/instances/9b59154ae666471993659902ed0ba749/raiseEvent/{{eventName}}?taskHub=SampleHubVS&connection=Storage&code=mykey", + (string)status["sendEventPostUri"]); + Assert.Equal( + $"{TestConstants.NotificationUrlBase}/instances/9b59154ae666471993659902ed0ba749/terminate?reason={{text}}&taskHub=SampleHubVS&connection=Storage&code=mykey", + (string)status["terminatePostUri"]); + Assert.Equal( + $"{TestConstants.NotificationUrlBase}/instances/9b59154ae666471993659902ed0ba749?taskHub=SampleHubVS&connection=Storage&code=mykey", + (string)status["purgeHistoryDeleteUri"]); + Assert.True(stopWatch.Elapsed > TimeSpan.FromSeconds(30)); + } + [Fact] [Trait("Category", PlatformSpecificHelpers.TestCategory)] public async Task WaitForCompletionOrCreateCheckStatusResponseAsync_Returns_HTTP_202_Response_After_Timeout() @@ -808,7 +848,7 @@ public async Task StartNewInstanceAndWaitToComplete_Is_Success(string instanceId .Returns(Task.FromResult(testInstanceId)); clientMock - .Setup(x => x.WaitForCompletionOrCreateCheckStatusResponseAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Setup(x => x.WaitForCompletionOrCreateCheckStatusResponseAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(Task.FromResult(testResponse)); var httpApiHandler = new ExtendedHttpApiHandler(clientMock.Object); From 20a671e858e9e61c138da327d0c0b26dfdec4909 Mon Sep 17 00:00:00 2001 From: amdeel <52223332+amdeel@users.noreply.github.com> Date: Wed, 18 Nov 2020 16:06:11 -0800 Subject: [PATCH 2/4] updated HandleGetStatusRequestAsync in HttpApiHandler --- src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs index a1ca05e32..d356f7d08 100644 --- a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs +++ b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs @@ -196,7 +196,7 @@ internal async Task WaitForCompletionOrCreateCheckStatusRes status.RuntimeStatus == OrchestrationRuntimeStatus.Failed || status.RuntimeStatus == OrchestrationRuntimeStatus.Terminated) { - return await this.HandleGetStatusRequestAsync(request, instanceId); + return await this.HandleGetStatusRequestAsync(request, instanceId, returnInternalServerErrorOnFailure); } } @@ -519,7 +519,8 @@ private async Task HandleDeleteHistoryWithFiltersRequestAsy private async Task HandleGetStatusRequestAsync( HttpRequestMessage request, - string instanceId) + string instanceId, + bool returnInternalServerErrorOnFailure = false) { IDurableOrchestrationClient client = this.GetClient(request); var queryNameValuePairs = request.GetQueryNameValuePairs(); @@ -539,9 +540,12 @@ private async Task HandleGetStatusRequestAsync( showInput = true; } - if (!TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out bool returnInternalServerErrorOnFailure)) + if (returnInternalServerErrorOnFailure == false) { - returnInternalServerErrorOnFailure = false; + if (!TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out returnInternalServerErrorOnFailure)) + { + returnInternalServerErrorOnFailure = false; + } } var status = await client.GetStatusAsync(instanceId, showHistory, showHistoryOutput, showInput); From 60b42d34ddf9d312b24b3f9efca9de1784cd3868 Mon Sep 17 00:00:00 2001 From: amdeel <52223332+amdeel@users.noreply.github.com> Date: Fri, 20 Nov 2020 11:29:04 -0800 Subject: [PATCH 3/4] updated bool to bool? --- src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs index d356f7d08..6890b2e16 100644 --- a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs +++ b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs @@ -520,7 +520,7 @@ private async Task HandleDeleteHistoryWithFiltersRequestAsy private async Task HandleGetStatusRequestAsync( HttpRequestMessage request, string instanceId, - bool returnInternalServerErrorOnFailure = false) + bool? returnInternalServerErrorOnFailure = null) { IDurableOrchestrationClient client = this.GetClient(request); var queryNameValuePairs = request.GetQueryNameValuePairs(); @@ -540,9 +540,13 @@ private async Task HandleGetStatusRequestAsync( showInput = true; } - if (returnInternalServerErrorOnFailure == false) + if (returnInternalServerErrorOnFailure == null) { - if (!TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out returnInternalServerErrorOnFailure)) + if (TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out bool returnInternalServerErrorParameter)) + { + returnInternalServerErrorOnFailure = returnInternalServerErrorParameter; + } + else { returnInternalServerErrorOnFailure = false; } @@ -569,7 +573,7 @@ private async Task HandleGetStatusRequestAsync( // The orchestration has failed - return 500 w/out Location header case OrchestrationRuntimeStatus.Failed: - statusCode = returnInternalServerErrorOnFailure ? HttpStatusCode.InternalServerError : HttpStatusCode.OK; + statusCode = (bool)returnInternalServerErrorOnFailure ? HttpStatusCode.InternalServerError : HttpStatusCode.OK; location = null; break; From 78bb373c80ac9c635c709dc72502c9293c700116 Mon Sep 17 00:00:00 2001 From: amdeel <52223332+amdeel@users.noreply.github.com> Date: Tue, 24 Nov 2020 14:55:48 -0800 Subject: [PATCH 4/4] modified returnInternalServerErrorOnFailure on HandleGetStatusRequestAsync --- .../HttpApiHandler.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs index 6890b2e16..ed138bf17 100644 --- a/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs +++ b/src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs @@ -540,15 +540,16 @@ private async Task HandleGetStatusRequestAsync( showInput = true; } - if (returnInternalServerErrorOnFailure == null) + bool finalReturnInternalServerErrorOnFailure; + if (returnInternalServerErrorOnFailure.HasValue) { - if (TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out bool returnInternalServerErrorParameter)) - { - returnInternalServerErrorOnFailure = returnInternalServerErrorParameter; - } - else + finalReturnInternalServerErrorOnFailure = returnInternalServerErrorOnFailure.Value; + } + else + { + if (!TryGetBooleanQueryParameterValue(queryNameValuePairs, ReturnInternalServerErrorOnFailure, out finalReturnInternalServerErrorOnFailure)) { - returnInternalServerErrorOnFailure = false; + finalReturnInternalServerErrorOnFailure = false; } } @@ -573,7 +574,7 @@ private async Task HandleGetStatusRequestAsync( // The orchestration has failed - return 500 w/out Location header case OrchestrationRuntimeStatus.Failed: - statusCode = (bool)returnInternalServerErrorOnFailure ? HttpStatusCode.InternalServerError : HttpStatusCode.OK; + statusCode = finalReturnInternalServerErrorOnFailure ? HttpStatusCode.InternalServerError : HttpStatusCode.OK; location = null; break;