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

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,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using Microsoft.DurableTask.Abstractions;
+
 namespace Microsoft.DurableTask;
 
 /// <summary>
@@ -20,6 +22,12 @@ public class RetryPolicy
     /// The maximum time to delay between attempts, regardless of<paramref name="backoffCoefficient"/>.
     /// </param>
     /// <param name="retryTimeout">The overall timeout for retries.</param>
+    /// <param name="handle">
+    /// 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.
+    /// </param>
     /// <remarks>
     /// The value <see cref="Timeout.InfiniteTimeSpan"/> can be used to specify an unlimited timeout for
     /// <paramref name="maxRetryInterval"/> or <paramref name="retryTimeout"/>.
@@ -39,7 +47,8 @@ public RetryPolicy(
         TimeSpan firstRetryInterval,
         double backoffCoefficient = 1.0,
         TimeSpan? maxRetryInterval = null,
-        TimeSpan? retryTimeout = null)
+        TimeSpan? retryTimeout = null,
+        Func<TaskFailureDetails, bool>? handle = null)
     {
         if (maxNumberOfAttempts <= 0)
         {
@@ -86,6 +95,7 @@ public RetryPolicy(
         this.BackoffCoefficient = backoffCoefficient;
         this.MaxRetryInterval = maxRetryInterval ?? TimeSpan.FromHours(1);
         this.RetryTimeout = retryTimeout ?? Timeout.InfiniteTimeSpan;
+        this.Handle = CreateHandler(handle);
     }
 
     /// <summary>
@@ -123,11 +133,56 @@ public RetryPolicy(
     /// </value>
     public TimeSpan RetryTimeout { get; }
 
+    /// <summary>
+    /// Gets a delegate to call on exception to determine if retries should proceed.
+    /// </summary>
+    /// <value>
+    /// Defaults delegate that always returns true (i.e., all exceptions are retried).
+    /// </value>
+    public Func<Exception, bool> Handle { get; private set; }
+
 #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.")]
     public Func<Exception, Task<bool>>? HandleAsync { get; set; }
 #pragma warning restore SA1623 // Property summary documentation should match accessors
+
+    /// <summary>
+    /// Create delegate for <see cref="Handle"/> property.
+    /// </summary>
+    /// <param name="handle">
+    /// Deletegate that receives <see cref="TaskFailedException"/> and returns boolean that
+    /// determines if the task should be retried.
+    /// </param>
+    /// <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>
+    static Func<Exception, bool> CreateHandler(Func<TaskFailureDetails, bool>? handle)
+    {
+        return handle is null
+            ? ((ex) => true)
+            : ((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 handle.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,66 @@ 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,
+            handle: 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 +329,76 @@ 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,
+            handle: 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