Add RetryPolicy.Handle Property to Allow for Exception Filtering on …

…Retries (#314)

Co-authored-by: Tom Seida <[email protected]>

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`

src/Abstractions/DurableTaskCoreExceptionsExtensions.cs

@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Microsoft.DurableTask.Abstractions;
+
+/// <summary>
+/// Extension methods realated to the global::DurableTask.Core namespace items.
+/// </summary>
+static class DurableTaskCoreExceptionsExtensions
+{
+    /// <summary>
+    /// Converts <paramref name="taskFailedException"/> to a <see cref="TaskFailureDetails"/> instance.
+    /// If <paramref name="taskFailedException"/> does not contain FailureDetails, null shall be returned.
+    /// </summary>
+    /// <param name="taskFailedException"><see cref="global::DurableTask.Core.Exceptions.TaskFailedException"/> instance.</param>
+    /// <returns>
+    /// A <see cref="TaskFailureDetails"/> instance if <paramref name="taskFailedException"/> contains
+    /// FailureDetails; otherwise, null is returned.
+    /// </returns>
+    internal static TaskFailureDetails? ToTaskFailureDetails(this global::DurableTask.Core.Exceptions.TaskFailedException taskFailedException)
+        => taskFailedException.FailureDetails.ToTaskFailureDetails();
+
+    /// <summary>
+    /// Converts <paramref name="subOrchestrationFailedException"/> to a <see cref="TaskFailureDetails"/> instance.
+    /// If <paramref name="subOrchestrationFailedException"/> does not contain FailureDetails, null shall be returned.
+    /// </summary>
+    /// <param name="subOrchestrationFailedException"><see cref="global::DurableTask.Core.Exceptions.SubOrchestrationFailedException"/> instance.</param>
+    /// <returns>
+    /// A <see cref="TaskFailureDetails"/> instance if <paramref name="subOrchestrationFailedException"/> contains
+    /// FailureDetails; otherwise, null is returned.
+    /// </returns>
+    internal static TaskFailureDetails? ToTaskFailureDetails(this global::DurableTask.Core.Exceptions.SubOrchestrationFailedException subOrchestrationFailedException) => subOrchestrationFailedException.FailureDetails.ToTaskFailureDetails();
+
+    /// <summary>
+    /// Converts <paramref name="failureDetails"/> to a <see cref="TaskFailureDetails"/> instance.
+    /// </summary>
+    /// <param name="failureDetails"><see cref="global::DurableTask.Core.FailureDetails"/> instance.</param>
+    /// <returns>
+    /// A <see cref="TaskFailureDetails"/> instance if <paramref name="failureDetails"/> is not null; otherwise, null.
+    /// </returns>
+    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());
+    }
+}

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;
 
 /// <summary>
@@ -86,6 +89,7 @@ public RetryPolicy(
         this.BackoffCoefficient = backoffCoefficient;
         this.MaxRetryInterval = maxRetryInterval ?? TimeSpan.FromHours(1);
         this.RetryTimeout = retryTimeout ?? Timeout.InfiniteTimeSpan;
+        this.Handle = (ex) => true;
     }
 
     /// <summary>
@@ -123,11 +127,58 @@ public RetryPolicy(
     /// </value>
     public TimeSpan RetryTimeout { get; }
 
+    /// <summary>
+    /// Gets a delegate to call on exception to determine if retries should proceed.
+    /// For internal usage, use <see cref="HandleFailure" /> for setting this delegate.
+    /// </summary>
+    /// <value>
+    /// Defaults delegate that always returns true (i.e., all exceptions are retried).
+    /// </value>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public Func<Exception, bool> Handle { get; private init; }
+
 #pragma warning disable SA1623 // Property summary documentation should match accessors
     /// <summary>
     /// This functionality is not implemented. Will be removed in the future. Use TaskOptions.FromRetryHandler instead.
     /// </summary>
-    [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<Exception, Task<bool>>? HandleAsync { get; set; }
 #pragma warning restore SA1623 // Property summary documentation should match accessors
+
+    /// <summary>
+    /// Optional delegate to invoke on exceptions to determine if retries should proceed. The delegate shall receive a
+    /// <see cref="TaskFailureDetails"/> 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.
+    /// </summary>
+    /// <exception cref="InvalidOperationException">
+    /// This represents a defect in this library in that it should always receive either
+    /// <see cref="global::DurableTask.Core.Exceptions.TaskFailedException"/> or
+    /// <see cref="global::DurableTask.Core.Exceptions.SubOrchestrationFailedException"/>.
+    /// </exception>
+    public Func<TaskFailureDetails, bool> 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);
+                };
+        }
+    }
 }

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,
         };
     }
 }

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);
+    }
+
     /// <summary>
     /// Tests retry policies for sub-orchestration calls.
     /// </summary>
@@ -269,6 +331,78 @@ public async Task RetrySubOrchestrationFailures(int expectedNumberOfAttempts, Ty
         Assert.True(metadata.FailureDetails.IsCausedBy<TaskFailedException>());
     }
 
+    [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<TaskFailedException>());
+        }
+        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