update by comments

Author: nytian

src/DurableTask.Core/ExceptionPropertiesProviderExtensions.cs

@@ -0,0 +1,46 @@
+//  ----------------------------------------------------------------------------------
+//  Copyright Microsoft Corporation
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//  http://www.apache.org/licenses/LICENSE-2.0
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+//  ----------------------------------------------------------------------------------
+#nullable enable
+namespace DurableTask.Core
+{
+    using System;
+    using System.Collections.Generic;
+    using DurableTask.Core.Exceptions;
+
+    /// <summary>
+    /// Extension methods for <see cref="IExceptionPropertiesProvider"/>.
+    /// </summary>
+    internal static class ExceptionPropertiesProviderExtensions
+    {
+        /// <summary>
+        /// Extracts properties for the specified rules with the provider.
+        /// </summary>
+        public static IDictionary<string, object>? ExtractProperties(this IExceptionPropertiesProvider? provider, Exception exception)
+        {
+            if (exception is OrchestrationException orchestrationException &&
+                orchestrationException.FailureDetails?.Properties != null)
+            {
+                return orchestrationException.FailureDetails.Properties;
+            }
+
+            if (provider == null)
+            {
+                return null;
+            }
+
+            return provider.GetExceptionProperties(exception);
+        }
+    }
+}
+
+

src/DurableTask.Core/FailureDetails.cs

@@ -38,14 +38,29 @@ public class FailureDetails : IEquatable<FailureDetails>
         /// <param name="stackTrace">The exception stack trace.</param>
         /// <param name="innerFailure">The inner cause of the failure.</param>
         /// <param name="isNonRetriable">Whether the failure is non-retriable.</param>
+        /// <param name="properties">Additional properties associated with the failure.</param>
         [JsonConstructor]
-        public FailureDetails(string errorType, string errorMessage, string? stackTrace, FailureDetails? innerFailure, bool isNonRetriable)
+        public FailureDetails(string errorType, string errorMessage, string? stackTrace, FailureDetails? innerFailure, bool isNonRetriable, IDictionary<string, object>? properties = null)
         {
             this.ErrorType = errorType;
             this.ErrorMessage = errorMessage;
             this.StackTrace = stackTrace;
             this.InnerFailure = innerFailure;
             this.IsNonRetriable = isNonRetriable;
+            this.Properties = properties;
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="FailureDetails"/> class.
+        /// </summary>
+        /// <param name="errorType">The name of the error, which is expected to the the namespace-qualified name of the exception type.</param>
+        /// <param name="errorMessage">The message associated with the error, which is expected to be the exception's <see cref="Exception.Message"/> property.</param>
+        /// <param name="stackTrace">The exception stack trace.</param>
+        /// <param name="innerFailure">The inner cause of the failure.</param>
+        /// <param name="isNonRetriable">Whether the failure is non-retriable.</param>
+        public FailureDetails(string errorType, string errorMessage, string? stackTrace, FailureDetails? innerFailure, bool isNonRetriable)
+            : this(errorType, errorMessage, stackTrace, innerFailure, isNonRetriable, properties:null)
+        {
         }
 
         /// <summary>
@@ -54,7 +69,7 @@ public FailureDetails(string errorType, string errorMessage, string? stackTrace,
         /// <param name="e">The exception used to generate the failure details.</param>
         /// <param name="innerFailure">The inner cause of the failure.</param>
         public FailureDetails(Exception e, FailureDetails innerFailure)
-            : this(e, innerFailure, null)
+            : this(e, innerFailure, properties: null)
         {
         }
 
@@ -63,31 +78,29 @@ public FailureDetails(Exception e, FailureDetails innerFailure)
         /// </summary>
         /// <param name="e">The exception used to generate the failure details.</param>
         public FailureDetails(Exception e)
-            : this(e, (IExceptionPropertiesProvider?)null)
+            : this(e, properties: null)
         {
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="FailureDetails"/> class from an exception object.
         /// </summary>
         /// <param name="e">The exception used to generate the failure details.</param>
-        /// <param name="exceptionPropertiesProvider">The provider to extract custom properties from the exception.</param>
-        public FailureDetails(Exception e, IExceptionPropertiesProvider? exceptionPropertiesProvider)
-            : this(e.GetType().FullName, GetErrorMessage(e), e.StackTrace, FromException(e.InnerException, exceptionPropertiesProvider), false)
+        /// <param name="properties">The exception properties to include in failure details.</param> 
+        public FailureDetails(Exception e, IDictionary<string, object>? properties)
+            : this(e.GetType().FullName, GetErrorMessage(e), e.StackTrace, FromException(e.InnerException), false, properties)
         {
-            this.Properties = GetExceptionProperties(e, exceptionPropertiesProvider);
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="FailureDetails"/> class from an exception object.
         /// </summary>
         /// <param name="e">The exception used to generate the failure details.</param>
         /// <param name="innerFailure">The inner cause of the failure.</param>
-        /// <param name="exceptionPropertiesProvider">The provider to extract custom properties from the exception.</param>
-        public FailureDetails(Exception e, FailureDetails innerFailure, IExceptionPropertiesProvider? exceptionPropertiesProvider)
-            : this(e.GetType().FullName, GetErrorMessage(e), e.StackTrace, innerFailure, false)
+        /// <param name="properties">The exception properties to include in failure details.</param> 
+        public FailureDetails(Exception e, FailureDetails innerFailure, IDictionary<string, object>? properties)
+            : this(e.GetType().FullName, GetErrorMessage(e), e.StackTrace, innerFailure, false, properties)
         {
-            this.Properties = GetExceptionProperties(e, exceptionPropertiesProvider);
         }
 
         /// <summary>
@@ -243,31 +256,12 @@ static string GetErrorMessage(Exception e)
 
         static FailureDetails? FromException(Exception? e)
         {
-            return FromException(e, null);
-        }
-
-        static FailureDetails? FromException(Exception? e, IExceptionPropertiesProvider? provider)
-        {
-            return e == null ? null : new FailureDetails(e, provider);
+            return FromException(e, properties : null);
         }
 
-        static IDictionary<string, object>? GetExceptionProperties(Exception exception, IExceptionPropertiesProvider? provider)
+        static FailureDetails? FromException(Exception? e, IDictionary<string, object>? properties)
         {
-            // If this is a TaskFailedException that already has FailureDetails with properties,
-            // use those properties instead of asking the provider
-            if (exception is OrchestrationException orchestrationException && 
-                orchestrationException.FailureDetails?.Properties != null)
-            {
-                return orchestrationException.FailureDetails.Properties;
-            }
-
-            if (provider == null)
-            {
-                return null;
-            }
-
-            // If there is a provider provided, then extract exception properties with the provider.
-            return provider.GetExceptionProperties(exception);
+            return e == null ? null : new FailureDetails(e, properties : properties);
         }
 
     }

src/DurableTask.Core/ReflectionBasedTaskActivity.cs

@@ -140,7 +140,8 @@ public override async Task<string> RunAsync(TaskContext context, string input)
                 }
                 else
                 {
-                    failureDetails = new FailureDetails(exception, context.ExceptionPropertiesProvider);
+                    var props = context.ExceptionPropertiesProvider.ExtractProperties(exception);
+                    failureDetails = new FailureDetails(exception, props);
                 }
 
                 throw new TaskFailureException(exception.Message, exception, details)

src/DurableTask.Core/TaskActivity.cs

@@ -13,12 +13,12 @@
 
 namespace DurableTask.Core
 {
-    using System;
-    using System.Threading.Tasks;
     using DurableTask.Core.Common;
     using DurableTask.Core.Exceptions;
     using DurableTask.Core.Serializing;
     using Newtonsoft.Json.Linq;
+    using System;
+    using System.Threading.Tasks;
 
     /// <summary>
     ///     Base class for TaskActivity.
@@ -144,7 +144,8 @@ public override async Task<string> RunAsync(TaskContext context, string input)
                 {
                     if(context != null)
                     {
-                        failureDetails = new FailureDetails(e, context.ExceptionPropertiesProvider);
+                        var props = context.ExceptionPropertiesProvider.ExtractProperties(e);
+                        failureDetails = new FailureDetails(e, props);
                     }
                     else
                     {

src/DurableTask.Core/TaskActivityDispatcher.cs

@@ -38,16 +38,6 @@ public sealed class TaskActivityDispatcher
         readonly ErrorPropagationMode errorPropagationMode;
         readonly IExceptionPropertiesProvider? exceptionPropertiesProvider;
 
-        internal TaskActivityDispatcher(
-            IOrchestrationService orchestrationService,
-            INameVersionObjectManager<TaskActivity> objectManager,
-            DispatchMiddlewarePipeline dispatchPipeline,
-            LogHelper logHelper,
-            ErrorPropagationMode errorPropagationMode)
-            : this(orchestrationService, objectManager, dispatchPipeline, logHelper, errorPropagationMode, null)
-        {
-        }
-
         /// <summary>
         /// Initializes a new instance of the <see cref="TaskActivityDispatcher"/> class with an exception properties provider.
         /// </summary>
@@ -230,7 +220,7 @@ await this.dispatchPipeline.RunAsync(dispatchContext, async _ =>
                             string? details = this.IncludeDetails
                                 ? $"Unhandled exception while executing task: {e}"
                                 : null;
-                            responseEvent = new TaskFailedEvent(-1, scheduledEvent.EventId, e.Message, details, new FailureDetails(e, this.exceptionPropertiesProvider));
+                            responseEvent = new TaskFailedEvent(-1, scheduledEvent.EventId, e.Message, details, new FailureDetails(e));
 
                             traceActivity?.SetStatus(ActivityStatusCode.Error, e.Message);
 

src/DurableTask.Core/TaskEntityDispatcher.cs

@@ -44,16 +44,6 @@ public class TaskEntityDispatcher
         readonly TaskOrchestrationDispatcher.NonBlockingCountdownLock concurrentSessionLock;
         readonly IExceptionPropertiesProvider exceptionPropertiesProvider;
 
-        internal TaskEntityDispatcher(
-            IOrchestrationService orchestrationService,
-            INameVersionObjectManager<TaskEntity> entityObjectManager,
-            DispatchMiddlewarePipeline entityDispatchPipeline,
-            LogHelper logHelper,
-            ErrorPropagationMode errorPropagationMode)
-            : this(orchestrationService, entityObjectManager, entityDispatchPipeline, logHelper, errorPropagationMode, null)
-        {
-        }
-
         /// <summary>
         /// Initializes a new instance of the <see cref="TaskEntityDispatcher"/> class with an exception properties provider.
         /// </summary>

src/DurableTask.Core/TaskOrchestration.cs

@@ -105,7 +105,8 @@ public override async Task<string> Execute(OrchestrationContext context, string
                 }
                 else
                 {
-                    failureDetails = new FailureDetails(e, context.ExceptionPropertiesProvider);
+                    var props = context.ExceptionPropertiesProvider.ExtractProperties(e);
+                    failureDetails = new FailureDetails(e, props);
                 }
 
                 throw new OrchestrationFailureException(e.Message, details)

src/DurableTask.Core/TaskOrchestrationContext.cs

@@ -52,17 +52,8 @@ public TaskOrchestrationContext(
             OrchestrationInstance orchestrationInstance,
             TaskScheduler taskScheduler,
             TaskOrchestrationEntityParameters entityParameters = null,
-            ErrorPropagationMode errorPropagationMode = ErrorPropagationMode.SerializeExceptions)
-            : this(orchestrationInstance, taskScheduler, entityParameters, errorPropagationMode, null)
-        {
-        }
-
-        public TaskOrchestrationContext(
-            OrchestrationInstance orchestrationInstance,
-            TaskScheduler taskScheduler,
-            TaskOrchestrationEntityParameters entityParameters,
-            ErrorPropagationMode errorPropagationMode,
-            IExceptionPropertiesProvider exceptionPropertiesProvider)
+            ErrorPropagationMode errorPropagationMode = ErrorPropagationMode.SerializeExceptions,
+            IExceptionPropertiesProvider exceptionPropertiesProvider = null)
         {
             Utils.UnusedParameter(taskScheduler);
 
@@ -696,7 +687,7 @@ public void FailOrchestration(Exception failure, OrchestrationRuntimeState runti
             {
                 if (this.ErrorPropagationMode == ErrorPropagationMode.UseFailureDetails)
                 {
-                    failureDetails = new FailureDetails(failure, this.exceptionPropertiesProvider);
+                    failureDetails = new FailureDetails(failure);
                 }
                 else
                 {

src/DurableTask.Core/TaskOrchestrationDispatcher.cs

@@ -51,17 +51,6 @@ public class TaskOrchestrationDispatcher
         readonly VersioningSettings? versioningSettings;
         readonly IExceptionPropertiesProvider? exceptionPropertiesProvider;
 
-        internal TaskOrchestrationDispatcher(
-            IOrchestrationService orchestrationService,
-            INameVersionObjectManager<TaskOrchestration> objectManager,
-            DispatchMiddlewarePipeline dispatchPipeline,
-            LogHelper logHelper,
-            ErrorPropagationMode errorPropagationMode,
-            VersioningSettings versioningSettings)
-            : this(orchestrationService, objectManager, dispatchPipeline, logHelper, errorPropagationMode, versioningSettings, null)
-        {
-        }
-
         /// <summary>
         /// Initializes a new instance of the <see cref="TaskOrchestrationDispatcher"/> class with an exception properties provider.
         /// </summary>

test/DurableTask.Core.Tests/TestTaskEntityDispatcher.cs

@@ -30,7 +30,7 @@ private TaskEntityDispatcher GetTaskEntityDispatcher()
             var logger = new LogHelper(loggerFactory?.CreateLogger("DurableTask.Core"));
 
             TaskEntityDispatcher dispatcher = new TaskEntityDispatcher(
-                service, entityManager, entityMiddleware, logger, ErrorPropagationMode.UseFailureDetails);
+                service, entityManager, entityMiddleware, logger, ErrorPropagationMode.UseFailureDetails, null);
             return dispatcher;
         }