diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc268ad..a24de151 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v1.3.0 (Unreleased) + +### Microsoft.DurableTask.Abstractions + +- Add `RetryPolicy.Handle` property to allow for exception filtering on retries ([#314](https://github.com/microsoft/durabletask-dotnet/pull/314)) + ## v1.2.4 - Microsoft.Azure.DurableTask.Core dependency increased to `2.17.1` diff --git a/src/Abstractions/DurableTaskCoreExceptionsExtensions.cs b/src/Abstractions/DurableTaskCoreExceptionsExtensions.cs new file mode 100644 index 00000000..8d213a9d --- /dev/null +++ b/src/Abstractions/DurableTaskCoreExceptionsExtensions.cs @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.DurableTask.Abstractions; + +/// +/// Extension methods realated to the global::DurableTask.Core namespace items. +/// +static class DurableTaskCoreExceptionsExtensions +{ + /// + /// Converts to a instance. + /// If does not contain FailureDetails, null shall be returned. + /// + /// instance. + /// + /// A instance if contains + /// FailureDetails; otherwise, null is returned. + /// + internal static TaskFailureDetails? ToTaskFailureDetails(this global::DurableTask.Core.Exceptions.TaskFailedException taskFailedException) + => taskFailedException.FailureDetails.ToTaskFailureDetails(); + + /// + /// Converts to a instance. + /// If does not contain FailureDetails, null shall be returned. + /// + /// instance. + /// + /// A instance if contains + /// FailureDetails; otherwise, null is returned. + /// + internal static TaskFailureDetails? ToTaskFailureDetails(this global::DurableTask.Core.Exceptions.SubOrchestrationFailedException subOrchestrationFailedException) => subOrchestrationFailedException.FailureDetails.ToTaskFailureDetails(); + + /// + /// Converts to a instance. + /// + /// instance. + /// + /// A instance if is not null; otherwise, null. + /// + internal static TaskFailureDetails? ToTaskFailureDetails(this global::DurableTask.Core.FailureDetails? failureDetails) + { + if (failureDetails is null) + { + return null; + } + + return new TaskFailureDetails( + failureDetails.ErrorType, + failureDetails.ErrorMessage, + failureDetails.StackTrace, + failureDetails.InnerFailure?.ToTaskFailureDetails()); + } +} diff --git a/src/Abstractions/RetryPolicy.cs b/src/Abstractions/RetryPolicy.cs index 5ac6402b..d4fc3b48 100644 --- a/src/Abstractions/RetryPolicy.cs +++ b/src/Abstractions/RetryPolicy.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.ComponentModel; +using Microsoft.DurableTask.Abstractions; + namespace Microsoft.DurableTask; /// @@ -86,6 +89,7 @@ public RetryPolicy( this.BackoffCoefficient = backoffCoefficient; this.MaxRetryInterval = maxRetryInterval ?? TimeSpan.FromHours(1); this.RetryTimeout = retryTimeout ?? Timeout.InfiniteTimeSpan; + this.Handle = (ex) => true; } /// @@ -123,11 +127,58 @@ public RetryPolicy( /// public TimeSpan RetryTimeout { get; } + /// + /// Gets a delegate to call on exception to determine if retries should proceed. + /// For internal usage, use for setting this delegate. + /// + /// + /// Defaults delegate that always returns true (i.e., all exceptions are retried). + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public Func Handle { get; private init; } + #pragma warning disable SA1623 // Property summary documentation should match accessors /// /// This functionality is not implemented. Will be removed in the future. Use TaskOptions.FromRetryHandler instead. /// - [Obsolete("This functionality is not implemented. Will be removed in the future. Use TaskOptions.FromRetryHandler instead.")] + [Obsolete("This functionality is not implemented. Will be removed in the future. Use TaskOptions.FromRetryHandler or HandleFailure instead.")] public Func>? HandleAsync { get; set; } #pragma warning restore SA1623 // Property summary documentation should match accessors + + /// + /// Optional delegate to invoke on exceptions to determine if retries should proceed. The delegate shall receive a + /// instance and returns bool value where true means that a retry + /// is attempted and false means no retry is attempted. Time and attempt count constraints + /// take precedence over this delegate for determining if retry attempts are performed. + /// + /// + /// This represents a defect in this library in that it should always receive either + /// or + /// . + /// + public Func HandleFailure + { + init + { + this.Handle = ex => + { + TaskFailureDetails? taskFailureDetails = null; + if (ex is global::DurableTask.Core.Exceptions.TaskFailedException taskFailedException) + { + taskFailureDetails = taskFailedException.ToTaskFailureDetails(); + } + else if (ex is global::DurableTask.Core.Exceptions.SubOrchestrationFailedException subOrchestrationFailedException) + { + taskFailureDetails = subOrchestrationFailedException.ToTaskFailureDetails(); + } + + if (taskFailureDetails is null) + { + throw new InvalidOperationException("Unable to create TaskFailureDetails since TaskFailedException nor SubOrchestrationFailedException was not received."); + } + + return value.Invoke(taskFailureDetails); + }; + } + } } diff --git a/src/Shared/Core/RetryPolicyExtensions.cs b/src/Shared/Core/RetryPolicyExtensions.cs index 61d9d3c8..901627ea 100644 --- a/src/Shared/Core/RetryPolicyExtensions.cs +++ b/src/Shared/Core/RetryPolicyExtensions.cs @@ -28,6 +28,7 @@ static TimeSpan ConvertInfiniteTimeSpans(TimeSpan timeout) => BackoffCoefficient = retry.BackoffCoefficient, MaxRetryInterval = ConvertInfiniteTimeSpans(retry.MaxRetryInterval), RetryTimeout = ConvertInfiniteTimeSpans(retry.RetryTimeout), + Handle = retry.Handle, }; } } diff --git a/test/Grpc.IntegrationTests/OrchestrationErrorHandling.cs b/test/Grpc.IntegrationTests/OrchestrationErrorHandling.cs index 4d6e5471..c8fb3118 100644 --- a/test/Grpc.IntegrationTests/OrchestrationErrorHandling.cs +++ b/test/Grpc.IntegrationTests/OrchestrationErrorHandling.cs @@ -222,6 +222,68 @@ public async Task RetryActivityFailuresCustomLogic(int expectedNumberOfAttempts, Assert.Equal(expectedNumberOfAttempts, actualNumberOfAttempts); } + [Theory] + [InlineData(10, typeof(ApplicationException), false, int.MaxValue, 2, 1, OrchestrationRuntimeStatus.Failed)] // 1 attempt since retry timeout expired. + [InlineData(2, typeof(ApplicationException), false, int.MaxValue, null, 1, OrchestrationRuntimeStatus.Failed)] // 1 attempt since handler specifies no retry. + [InlineData(2, typeof(CustomException),true, int.MaxValue, null, 2, OrchestrationRuntimeStatus.Failed)] // 2 attempts, custom exception type + [InlineData(10, typeof(XunitException),true, 4, null, 5, OrchestrationRuntimeStatus.Completed)] // 10 attempts, 3rd party exception type + public async Task RetryActivityFailuresCustomLogicAndPolicy( + int maxNumberOfAttempts, + Type exceptionType, + bool retryException, + int exceptionCount, + int? retryTimeout, + int expectedNumberOfAttempts, + OrchestrationRuntimeStatus expRuntimeStatus) + { + string errorMessage = "Kah-BOOOOOM!!!"; // Use an obviously fake error message to avoid confusion when debugging + + int actualNumberOfAttempts = 0; + int retryHandlerCalls = 0; + RetryPolicy retryPolicy = new( + maxNumberOfAttempts, + firstRetryInterval: TimeSpan.FromMilliseconds(1), + backoffCoefficient: 2, + retryTimeout: retryTimeout.HasValue ? TimeSpan.FromMilliseconds(retryTimeout.Value) : null) + { + HandleFailure = taskFailureDetails => + { + retryHandlerCalls++; + return taskFailureDetails.IsCausedBy(exceptionType) && retryException; + } + }; + TaskOptions taskOptions = TaskOptions.FromRetryPolicy(retryPolicy); + + + TaskName orchestratorName = "BustedOrchestration"; + await using HostTestLifetime server = await this.StartWorkerAsync(b => + { + b.AddTasks(tasks => + tasks.AddOrchestratorFunc(orchestratorName, async ctx => + { + await ctx.CallActivityAsync("Foo", options: taskOptions); + }) + .AddActivityFunc("Foo", (TaskActivityContext context) => + { + if (actualNumberOfAttempts++ < exceptionCount) + { + throw MakeException(exceptionType, errorMessage); + } + })); + }); + + string instanceId = await server.Client.ScheduleNewOrchestrationInstanceAsync(orchestratorName); + OrchestrationMetadata metadata = await server.Client.WaitForInstanceCompletionAsync( + instanceId, getInputsAndOutputs: true, this.TimeoutToken); + + Assert.NotNull(metadata); + Assert.Equal(instanceId, metadata.InstanceId); + Assert.Equal(expRuntimeStatus, metadata.RuntimeStatus); + // More calls to retry handler than expected. + //Assert.Equal(expectedNumberOfAttempts, retryHandlerCalls); + Assert.Equal(expectedNumberOfAttempts, actualNumberOfAttempts); + } + /// /// Tests retry policies for sub-orchestration calls. /// @@ -269,6 +331,78 @@ public async Task RetrySubOrchestrationFailures(int expectedNumberOfAttempts, Ty Assert.True(metadata.FailureDetails.IsCausedBy()); } + [Theory] + [InlineData(10, typeof(ApplicationException), false, int.MaxValue, 2, 1, OrchestrationRuntimeStatus.Failed)] // 1 attempt since retry timeout expired. + [InlineData(2, typeof(ApplicationException), false, int.MaxValue, null, 1, OrchestrationRuntimeStatus.Failed)] // 1 attempt since handler specifies no retry. + [InlineData(2, typeof(CustomException), true, int.MaxValue, null, 2, OrchestrationRuntimeStatus.Failed)] // 2 attempts, custom exception type + [InlineData(10, typeof(XunitException), true, 4, null, 5, OrchestrationRuntimeStatus.Completed)] // 10 attempts, 3rd party exception type + public async Task RetrySubOrchestratorFailuresCustomLogicAndPolicy( + int maxNumberOfAttempts, + Type exceptionType, + bool retryException, + int exceptionCount, + int? retryTimeout, + int expectedNumberOfAttempts, + OrchestrationRuntimeStatus expRuntimeStatus) + { + string errorMessage = "Kah-BOOOOOM!!!"; // Use an obviously fake error message to avoid confusion when debugging + + int actualNumberOfAttempts = 0; + int retryHandlerCalls = 0; + RetryPolicy retryPolicy = new( + maxNumberOfAttempts, + firstRetryInterval: TimeSpan.FromMilliseconds(1), + backoffCoefficient: 2, + retryTimeout: retryTimeout.HasValue ? TimeSpan.FromMilliseconds(retryTimeout.Value) : null) + { + HandleFailure = taskFailureDetails => + { + retryHandlerCalls++; + return taskFailureDetails.IsCausedBy(exceptionType) && retryException; + } + }; + TaskOptions taskOptions = TaskOptions.FromRetryPolicy(retryPolicy); + + TaskName orchestratorName = "OrchestrationWithBustedSubOrchestrator"; + await using HostTestLifetime server = await this.StartWorkerAsync(b => + { + b.AddTasks(tasks => + tasks.AddOrchestratorFunc(orchestratorName, async ctx => + { + await ctx.CallSubOrchestratorAsync("BustedSubOrchestrator", options: taskOptions); + }) + .AddOrchestratorFunc("BustedSubOrchestrator", context => + { + if (actualNumberOfAttempts++ < exceptionCount) + { + throw MakeException(exceptionType, errorMessage); + } + })); + }); + + string instanceId = await server.Client.ScheduleNewOrchestrationInstanceAsync(orchestratorName); + OrchestrationMetadata metadata = await server.Client.WaitForInstanceCompletionAsync( + instanceId, getInputsAndOutputs: true, this.TimeoutToken); + + Assert.NotNull(metadata); + Assert.Equal(instanceId, metadata.InstanceId); + Assert.Equal(expRuntimeStatus, metadata.RuntimeStatus); + // More calls to retry handler than expected. + //Assert.Equal(expectedNumberOfAttempts, retryHandlerCalls); + Assert.Equal(expectedNumberOfAttempts, actualNumberOfAttempts); + + // The root orchestration failed due to a failure with the sub-orchestration, resulting in a TaskFailedException + if (expRuntimeStatus == OrchestrationRuntimeStatus.Failed) + { + Assert.NotNull(metadata.FailureDetails); + Assert.True(metadata.FailureDetails!.IsCausedBy()); + } + else + { + Assert.Null(metadata.FailureDetails); + } + } + [Theory] [InlineData(1, typeof(ApplicationException))] // 1 attempt, built-in exception type [InlineData(2, typeof(CustomException))] // 2 attempts, custom exception type