Fix error handling propagation

CHANGELOG.md

@@ -10,6 +10,7 @@
 ### Updates
 
 * Fix integer overflow issues in GetScaleMetric and QueryManyOrchestrations ([#155](https://github.com/microsoft/durabletask-mssql/pull/155)) - contributed by [@bhugot](https://github.com/bhugot)
+* Fix error propagation to correctly expose detailed error info ([#188](https://github.com/microsoft/durabletask-mssql/pull/188))
 
 ## v1.1.1
 

src/DurableTask.SqlServer.AzureFunctions/DurableTask.SqlServer.AzureFunctions.csproj

@@ -16,7 +16,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.DurableTask" Version="2.9.*" />
+    <PackageReference Include="Microsoft.Azure.WebJobs.Extensions.DurableTask" Version="2.*" />
   </ItemGroup>
 
   <ItemGroup>

src/DurableTask.SqlServer/DTUtils.cs

@@ -5,9 +5,11 @@ namespace DurableTask.SqlServer
 {
     using System;
     using System.Diagnostics;
+    using System.IO;
     using System.Text;
     using DurableTask.Core;
     using DurableTask.Core.History;
+    using Newtonsoft.Json;
     using SemVersion;
 
     static class DTUtils
@@ -18,6 +20,13 @@ static class DTUtils
         // DurableTask.Core has a public static variable that contains the app name
         public static readonly string AppName = DurableTask.Core.Common.Utils.AppName;
 
+        static readonly JsonSerializer DefaultJsonSerializer = JsonSerializer.Create(
+            new JsonSerializerSettings
+            {
+                TypeNameHandling = TypeNameHandling.None,
+                NullValueHandling = NullValueHandling.Ignore,
+            });
+
         static SemanticVersion GetExtensionVersion()
         {
             // The product version is auto-generated by the Microsoft.SourceLink.GitHub nuget package.
@@ -98,18 +107,14 @@ static bool TryGetTaskScheduledId(HistoryEvent historyEvent, out int taskSchedul
             }
         }
 
-        public static int GetPayloadSizeInBytes(TaskMessage message)
+        public static bool TryGetPayloadText(HistoryEvent e, out string? payloadText)
         {
-            if (TryGetPayloadText(message.Event, out string? payloadText) && payloadText != null)
+            if (TryGetFailureDetails(e, out FailureDetails? failureDetails) && failureDetails != null)
             {
-                return Encoding.UTF8.GetByteCount(payloadText);
+                payloadText = SerializeToJson(failureDetails);
+                return true;
             }
 
-            return -1;
-        }
-
-        public static bool TryGetPayloadText(HistoryEvent e, out string? payloadText)
-        {
             payloadText = e.EventType switch
             {
                 EventType.ContinueAsNew => ((ContinueAsNewEvent)e).Result,
@@ -132,9 +137,24 @@ public static bool TryGetPayloadText(HistoryEvent e, out string? payloadText)
             return payloadText != null;
         }
 
+        public static bool TryGetFailureDetails(HistoryEvent e, out FailureDetails? failureDetails)
+        {
+            failureDetails = e.EventType switch
+            {
+                EventType.ExecutionCompleted => ((ExecutionCompletedEvent)e).FailureDetails,
+                EventType.ExecutionFailed => ((ExecutionCompletedEvent)e).FailureDetails,
+                EventType.TaskFailed => ((TaskFailedEvent)e).FailureDetails,
+                EventType.SubOrchestrationInstanceFailed => ((SubOrchestrationInstanceFailedEvent)e).FailureDetails,
+                _ => null,
+            };
+            return failureDetails != null;
+        }
+
         public static bool HasPayload(HistoryEvent e)
         {
-            return TryGetPayloadText(e, out string? text) && !string.IsNullOrEmpty(text);
+            return
+                (TryGetPayloadText(e, out string? text) && !string.IsNullOrEmpty(text)) ||
+                (TryGetFailureDetails(e, out FailureDetails? details) && details != null);
         }
 
         public static string? GetName(HistoryEvent historyEvent)
@@ -180,5 +200,52 @@ public static bool HasPayload(HistoryEvent e)
                 _ => null,
             };
         }
+
+        public static bool TryDeserializeFailureDetails(string jsonText, out FailureDetails? failureDetails)
+        {
+            try
+            {
+                failureDetails = DeserializeFromJson<FailureDetails>(jsonText);
+
+                // Depending on how the TaskHubWorker is configured, the text might be a FailureDetails JSON object
+                // or it might be a serialized Exception. Use the ErrorType field to distinguish. Note that we'll get a
+                // false-positive if the exception type has an ErrorType field so this is an imperfect check.
+                return failureDetails?.ErrorType != null;
+            }
+            catch (Exception)
+            {
+                failureDetails = null;
+                return false;
+            }
+        }
+
+        /// <summary>
+        /// Deserialize a JSON-string into an object of type T
+        /// This utility is resilient to end-user changes in the DefaultSettings of Newtonsoft.
+        /// </summary>
+        /// <typeparam name="T">The type to deserialize the JSON string into.</typeparam>
+        /// <param name="text">The JSON text.</param>
+        /// <returns>The deserialized value.</returns>
+        public static T DeserializeFromJson<T>(string text)
+        {
+            using var reader = new StringReader(text);
+            using var jsonReader = new JsonTextReader(reader);
+
+            return DefaultJsonSerializer.Deserialize<T>(jsonReader);
+        }
+
+        /// <summary>
+        /// Serializes an object to JSON text.
+        /// </summary>
+        /// <param name="obj">The object to serialize into JSON.</param>
+        /// <returns>The unformatted JSON text.</returns>
+        public static string SerializeToJson(object obj)
+        {
+            using var writer = new StringWriter();
+            DefaultJsonSerializer.Serialize(writer, obj);
+
+            writer.Flush();
+            return writer.ToString();
+        }
     }
 }

src/DurableTask.SqlServer/DbTaskEvent.cs

@@ -6,7 +6,7 @@ namespace DurableTask.SqlServer
     using System;
     using DurableTask.Core;
 
-    struct DbTaskEvent
+    readonly struct DbTaskEvent
     {
         readonly DateTime timestamp;
 

src/DurableTask.SqlServer/SqlOrchestrationService.cs

@@ -626,7 +626,7 @@ public override async Task<string> GetOrchestrationHistoryAsync(string instanceI
             using DbDataReader reader = await SqlUtils.ExecuteReaderAsync(command, this.traceHelper, instanceId);
 
             List<HistoryEvent> history = ReadHistoryEvents(reader, executionIdFilter);
-            return JsonConvert.SerializeObject(history);
+            return DTUtils.SerializeToJson(history);
         }
 
         static List<HistoryEvent> ReadHistoryEvents(

src/DurableTask.SqlServer/SqlUtils.cs

@@ -16,6 +16,7 @@ namespace DurableTask.SqlServer
     using DurableTask.Core.History;
     using Microsoft.Data.SqlClient;
     using Microsoft.Data.SqlClient.Server;
+    using Newtonsoft.Json;
     using SemVersion;
 
     static class SqlUtils
@@ -78,10 +79,18 @@ public static HistoryEvent GetHistoryEvent(this DbDataReader reader, bool isOrch
                         orchestrationStatus: OrchestrationStatus.Completed);
                     break;
                 case EventType.ExecutionFailed:
+                    string? executionFailedResult = null;
+                    if (!TryGetFailureDetails(reader, out FailureDetails? executionFailedDetails))
+                    {
+                        // Fall back to the old behavior
+                        executionFailedResult = GetPayloadText(reader);
+                    }
+
                     historyEvent = new ExecutionCompletedEvent(
                         eventId,
-                        result: GetPayloadText(reader),
-                        orchestrationStatus: OrchestrationStatus.Failed);
+                        result: executionFailedResult,
+                        orchestrationStatus: OrchestrationStatus.Failed,
+                        failureDetails: executionFailedDetails);
                     break;
                 case EventType.ExecutionStarted:
                     historyEvent = new ExecutionStartedEvent(eventId, GetPayloadText(reader))
@@ -133,11 +142,21 @@ public static HistoryEvent GetHistoryEvent(this DbDataReader reader, bool isOrch
                     };
                     break;
                 case EventType.SubOrchestrationInstanceFailed:
+                    string? subOrchFailedReason = null;
+                    string? subOrchFailedDetails = null;
+                    if (!TryGetFailureDetails(reader, out FailureDetails? subOrchFailureDetails))
+                    {
+                        // Fall back to the old behavior
+                        subOrchFailedReason = GetReason(reader);
+                        subOrchFailedDetails = GetPayloadText(reader);
+                    }
+
                     historyEvent = new SubOrchestrationInstanceFailedEvent(
                         eventId,
                         taskScheduledId: GetTaskId(reader),
-                        reason: GetReason(reader),
-                        details: GetPayloadText(reader));
+                        reason: subOrchFailedReason,
+                        details: subOrchFailedDetails,
+                        failureDetails: subOrchFailureDetails);
                     break;
                 case EventType.TaskCompleted:
                     historyEvent = new TaskCompletedEvent(
@@ -146,11 +165,21 @@ public static HistoryEvent GetHistoryEvent(this DbDataReader reader, bool isOrch
                         result: GetPayloadText(reader));
                     break;
                 case EventType.TaskFailed:
+                    string? taskFailedReason = null;
+                    string? taskFailedDetails = null;
+                    if (!TryGetFailureDetails(reader, out FailureDetails? taskFailureDetails))
+                    {
+                        // Fall back to the old behavior
+                        taskFailedReason = GetReason(reader);
+                        taskFailedDetails = GetPayloadText(reader);
+                    }
+
                     historyEvent = new TaskFailedEvent(
                         eventId,
                         taskScheduledId: GetTaskId(reader),
-                        reason: GetReason(reader),
-                        details: GetPayloadText(reader));
+                        reason: taskFailedReason,
+                        details: taskFailedDetails,
+                        failureDetails: taskFailureDetails);
                     break;
                 case EventType.TaskScheduled:
                     historyEvent = new TaskScheduledEvent(eventId)
@@ -182,6 +211,20 @@ public static HistoryEvent GetHistoryEvent(this DbDataReader reader, bool isOrch
             return historyEvent;
         }
 
+        static bool TryGetFailureDetails(DbDataReader reader, out FailureDetails? details)
+        {
+            // Failure details are expected to be in JSON format. In previous versions, it was just an error message
+            // that isn't expected to be JSON.
+            string? text = GetPayloadText(reader);
+            if (string.IsNullOrEmpty(text) || text![0] != '{')
+            {
+                details = null;
+                return false;
+            }
+
+            return DTUtils.TryDeserializeFailureDetails(text, out details);
+        }
+
         public static OrchestrationState GetOrchestrationState(this DbDataReader reader)
         {
             ParentInstance? parentInstance = null;
@@ -196,7 +239,8 @@ public static OrchestrationState GetOrchestrationState(this DbDataReader reader)
                     }
                 };
             }
-            return new OrchestrationState
+
+            var state = new OrchestrationState
             {
                 CompletedTime = reader.GetUtcDateTimeOrNull(reader.GetOrdinal("CompletedTime")) ?? default,
                 CreatedTime = reader.GetUtcDateTimeOrNull(reader.GetOrdinal("CreatedTime")) ?? default,
@@ -210,10 +254,27 @@ public static OrchestrationState GetOrchestrationState(this DbDataReader reader)
                     ExecutionId = GetExecutionId(reader),
                 },
                 OrchestrationStatus = GetRuntimeStatus(reader),
-                Output = GetStringOrNull(reader, reader.GetOrdinal("OutputText")),
                 Status = GetStringOrNull(reader, reader.GetOrdinal("CustomStatusText")),
                 ParentInstance = parentInstance
             };
+
+            // The OutputText column is overloaded to contain either orchestration output or failure details
+            // if the task hub worker is configured to use legacy error propagation.
+            string? rawOutput = GetStringOrNull(reader, reader.GetOrdinal("OutputText"));
+            if (rawOutput != null)
+            {
+                if (state.OrchestrationStatus == OrchestrationStatus.Failed &&
+                    DTUtils.TryDeserializeFailureDetails(rawOutput, out FailureDetails? failureDetails))
+                {
+                    state.FailureDetails = failureDetails;
+                }
+                else
+                {
+                    state.Output = rawOutput;
+                }
+            }
+
+            return state;
         }
 
         internal static DateTime? GetVisibleTime(HistoryEvent historyEvent)

test/DurableTask.SqlServer.Tests/Integration/DatabaseManagement.cs

@@ -316,7 +316,6 @@ public async Task CanCreateIfNotExists(bool isDatabaseMissing)
         /// <summary>
         /// Verifies that CreateIfNotExistsAsync is thread-safe.
         /// </summary>
-        /// <returns></returns>
         [Theory]
         [InlineData(true)]
         [InlineData(false)]