diff --git a/src/ConductorSharp.Client/ConductorSharp.Client.csproj b/src/ConductorSharp.Client/ConductorSharp.Client.csproj index 0bda519..5f4755c 100644 --- a/src/ConductorSharp.Client/ConductorSharp.Client.csproj +++ b/src/ConductorSharp.Client/ConductorSharp.Client.csproj @@ -6,7 +6,7 @@ Codaxy Codaxy ConductorSharp.Client - 4.1.0 + 4.2.0 Client library for Netflix Conductor, with some additional quality of life features. https://github.com/codaxy/conductor-sharp netflix;conductor diff --git a/src/ConductorSharp.Engine/ConductorSharp.Engine.csproj b/src/ConductorSharp.Engine/ConductorSharp.Engine.csproj index cae17be..86cc835 100644 --- a/src/ConductorSharp.Engine/ConductorSharp.Engine.csproj +++ b/src/ConductorSharp.Engine/ConductorSharp.Engine.csproj @@ -6,7 +6,7 @@ Codaxy Codaxy ConductorSharp.Engine - 4.1.0 + 4.2.0 Client library for Netflix Conductor, with some additional quality of life features. https://github.com/codaxy/conductor-sharp netflix;conductor diff --git a/src/ConductorSharp.Engine/Exceptions/StructuredErrorException.cs b/src/ConductorSharp.Engine/Exceptions/StructuredErrorException.cs index 86d1dd9..5a928f1 100644 --- a/src/ConductorSharp.Engine/Exceptions/StructuredErrorException.cs +++ b/src/ConductorSharp.Engine/Exceptions/StructuredErrorException.cs @@ -5,17 +5,22 @@ namespace ConductorSharp.Engine.Exceptions /// /// Thrown by a worker to attach a structured, sanitized error classification to the failed task's output. /// When caught by the execution manager, the // - /// are serialized under the structured_error output key (see + /// and the diagnostic message are serialized under the structured_error output key (see /// ), in addition to the plain /// error_message, so downstream consumers can read a stable classification without parsing free-text /// reasons. Plain exceptions are unaffected and keep producing only error_message. /// + /// + /// There is deliberately no Message property here: the diagnostic message is carried by the inherited + /// , which the message-taking constructors set. When no message is supplied it + /// falls back to , matching the behaviour of the original constructors. + /// public class StructuredErrorException : Exception { /// Stable, opaque classification code. Consumers map this to a failure response. public string Code { get; } - /// Human-readable, sanitized reason. Safe to surface across a layer boundary. + /// Short, stable, sanitized reason. Safe to surface across a layer boundary. public string Reason { get; } /// Optional URI pointing at the entity where the failure originated (drill-down link). @@ -36,5 +41,35 @@ public StructuredErrorException(string code, string reason, string referenceErro Reason = reason; ReferenceError = referenceError; } + + /// + /// Declares a diagnostic distinct from the short, stable + /// . Pass null for to fall back to the reason, + /// and null for when there is no entity to drill down into. + /// + /// + /// trails rather than following + /// on purpose. Overload resolution cannot choose between + /// this constructor and the one when the fourth argument is an untyped null, + /// so the nullable parameter is placed third, where it is typed the same either way. The only call this + /// leaves ambiguous is (code, reason, referenceError, null) — a declared-but-null inner exception, + /// which the three-argument constructor already expresses. Disambiguate with a named argument if needed. + /// + public StructuredErrorException(string code, string reason, string referenceError, string message) + : base(message ?? reason) + { + Code = code; + Reason = reason; + ReferenceError = referenceError; + } + + /// + public StructuredErrorException(string code, string reason, string referenceError, string message, Exception innerException) + : base(message ?? reason, innerException) + { + Code = code; + Reason = reason; + ReferenceError = referenceError; + } } } diff --git a/src/ConductorSharp.Engine/ExecutionManager.cs b/src/ConductorSharp.Engine/ExecutionManager.cs index 2ef451c..7eb1896 100644 --- a/src/ConductorSharp.Engine/ExecutionManager.cs +++ b/src/ConductorSharp.Engine/ExecutionManager.cs @@ -247,19 +247,9 @@ await _taskManager.UpdateAsync( pollResponse.WorkflowInstanceId ); - var errorMessage = new ErrorOutput { ErrorMessage = exception.Message }; - // A worker may throw a StructuredErrorException to attach a sanitized, stable classification to the // failed task's output. Plain exceptions keep producing only error_message, preserving backward compatibility. - if (exception is StructuredErrorException structuredException) - { - errorMessage.StructuredError = new StructuredError - { - Code = structuredException.Code, - Reason = structuredException.Reason, - ReferenceError = structuredException.ReferenceError - }; - } + var errorMessage = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) }; // TODO: We should verify that this is alright, it is possible that when executed concurrently, // the updates caused by LogAsync will be discarded because the call of UpdateAsync(TaskResult...) diff --git a/src/ConductorSharp.Engine/Model/StructuredError.cs b/src/ConductorSharp.Engine/Model/StructuredError.cs index 1457b0e..e61bbea 100644 --- a/src/ConductorSharp.Engine/Model/StructuredError.cs +++ b/src/ConductorSharp.Engine/Model/StructuredError.cs @@ -1,3 +1,6 @@ +using System; +using ConductorSharp.Engine.Exceptions; + namespace ConductorSharp.Engine.Model { /// @@ -12,13 +15,43 @@ public class StructuredError /// Stable, opaque classification code (e.g. an implementation-defined code, or UNCLASSIFIED). public string Code { get; set; } - /// Human-readable, sanitized reason. + /// Short, stable, sanitized reason. Consumers may key off this text, so keep it terse. public string Reason { get; set; } + /// + /// Optional diagnostic detail, longer and more specific than — the explanation an + /// operator needs, kept out of so that stays short and stable. Null when the producer + /// supplied nothing distinct from the reason, in which case it is omitted from serialized output + /// (NullValueHandling.Ignore) and the payload is unchanged from before this field existed. + /// + public string Message { get; set; } + /// Optional URI pointing at the entity where the failure originated (drill-down link). public string ReferenceError { get; set; } /// Payload shape version marker. Defaults to . public int Version { get; set; } = CurrentVersion; + + /// + /// Maps a thrown exception onto the payload, returning null for anything that is not a + /// so plain exceptions keep producing only error_message. + /// This is the single exception-to-payload mapping: both execution managers and the contract tests call it, + /// so the two poll strategies cannot drift apart as the shape evolves. + /// + public static StructuredError FromException(Exception exception) + { + if (exception is not StructuredErrorException structuredException) + return null; + + return new StructuredError + { + Code = structuredException.Code, + Reason = structuredException.Reason, + // Exception.Message falls back to Reason when the thrower supplied no distinct detail, so only + // carry it when it actually adds something. Existing call sites keep their exact payload. + Message = structuredException.Message == structuredException.Reason ? null : structuredException.Message, + ReferenceError = structuredException.ReferenceError + }; + } } } diff --git a/src/ConductorSharp.Engine/TypePollSpreadingExecutionManager.cs b/src/ConductorSharp.Engine/TypePollSpreadingExecutionManager.cs index 6fcab40..550bf1e 100644 --- a/src/ConductorSharp.Engine/TypePollSpreadingExecutionManager.cs +++ b/src/ConductorSharp.Engine/TypePollSpreadingExecutionManager.cs @@ -256,19 +256,9 @@ await _taskManager.UpdateAsync( pollResponse.WorkflowInstanceId ); - var errorMessage = new ErrorOutput { ErrorMessage = exception.Message }; - // A worker may throw a StructuredErrorException to attach a sanitized, stable classification to the // failed task's output. Plain exceptions keep producing only error_message, preserving backward compatibility. - if (exception is StructuredErrorException structuredException) - { - errorMessage.StructuredError = new StructuredError - { - Code = structuredException.Code, - Reason = structuredException.Reason, - ReferenceError = structuredException.ReferenceError - }; - } + var errorMessage = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) }; // TODO: We should verify that this is alright, it is possible that when executed concurrently, // the updates caused by LogAsync will be discarded because the call of UpdateAsync(TaskResult...) diff --git a/src/ConductorSharp.KafkaCancellationNotifier/ConductorSharp.KafkaCancellationNotifier.csproj b/src/ConductorSharp.KafkaCancellationNotifier/ConductorSharp.KafkaCancellationNotifier.csproj index ac791c8..fd0d8ab 100644 --- a/src/ConductorSharp.KafkaCancellationNotifier/ConductorSharp.KafkaCancellationNotifier.csproj +++ b/src/ConductorSharp.KafkaCancellationNotifier/ConductorSharp.KafkaCancellationNotifier.csproj @@ -4,7 +4,7 @@ net6.0 enable enable - 4.1.0 + 4.2.0 Codaxy Codaxy diff --git a/src/ConductorSharp.Patterns/ConductorSharp.Patterns.csproj b/src/ConductorSharp.Patterns/ConductorSharp.Patterns.csproj index bb0e862..c186b85 100644 --- a/src/ConductorSharp.Patterns/ConductorSharp.Patterns.csproj +++ b/src/ConductorSharp.Patterns/ConductorSharp.Patterns.csproj @@ -7,7 +7,7 @@ False Codaxy Codaxy - 4.1.0 + 4.2.0 diff --git a/src/ConductorSharp.Toolkit/ConductorSharp.Toolkit.csproj b/src/ConductorSharp.Toolkit/ConductorSharp.Toolkit.csproj index af3976e..f7e33f3 100644 --- a/src/ConductorSharp.Toolkit/ConductorSharp.Toolkit.csproj +++ b/src/ConductorSharp.Toolkit/ConductorSharp.Toolkit.csproj @@ -7,7 +7,7 @@ disable true dotnet-conductorsharp - 4.1.0 + 4.2.0 diff --git a/test/ConductorSharp.Engine.Tests/Unit/StructuredErrorTests.cs b/test/ConductorSharp.Engine.Tests/Unit/StructuredErrorTests.cs index 6fd7856..c4ab097 100644 --- a/test/ConductorSharp.Engine.Tests/Unit/StructuredErrorTests.cs +++ b/test/ConductorSharp.Engine.Tests/Unit/StructuredErrorTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using ConductorSharp.Client; using ConductorSharp.Client.Util; using ConductorSharp.Engine.Exceptions; @@ -12,40 +13,33 @@ namespace ConductorSharp.Engine.Tests.Unit { public class StructuredErrorTests { - // Mirrors the execution-manager catch block: builds the ErrorOutput (setting StructuredError for a - // StructuredErrorException) and serializes it. TryParse below asserts this output round-trips through the - // shared serializer, pinning the property-derived key/shape to StructuredErrorSerializer.OutputKey. + // Mirrors the execution-manager catch block. It calls StructuredError.FromException — the same mapping both + // ExecutionManager and TypePollSpreadingExecutionManager use — rather than reimplementing it, so a field + // added to the payload cannot pass here while being dropped by one of the managers. private static IDictionary SerializeCatchOutput(System.Exception exception) { - var output = new ErrorOutput { ErrorMessage = exception.Message }; - - if (exception is StructuredErrorException structuredException) - { - output.StructuredError = new StructuredError - { - Code = structuredException.Code, - Reason = structuredException.Reason, - ReferenceError = structuredException.ReferenceError - }; - } + var output = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) }; return SerializationHelper.ObjectToDictionary(output, ConductorConstants.IoJsonSerializerSettings); } + private static JToken StructuredErrorOf(IDictionary dict) => + JObject.Parse(JsonConvert.SerializeObject(dict))[StructuredErrorSerializer.OutputKey]; + [Fact] public void StructuredErrorException_produces_snake_case_structured_error() { - var exception = new StructuredErrorException("RESOURCE_UNAVAILABLE", "No port available", "https://rom/resourceOrder/42"); + var exception = new StructuredErrorException("RESOURCE_UNAVAILABLE", "No port available", "https://example.org/entity/42"); var dict = SerializeCatchOutput(exception); Assert.True(dict.ContainsKey("error_message")); Assert.True(dict.ContainsKey(StructuredErrorSerializer.OutputKey)); - var structured = JObject.Parse(JsonConvert.SerializeObject(dict))["structured_error"]; + var structured = StructuredErrorOf(dict); Assert.Equal("RESOURCE_UNAVAILABLE", (string)structured["code"]); Assert.Equal("No port available", (string)structured["reason"]); - Assert.Equal("https://rom/resourceOrder/42", (string)structured["reference_error"]); + Assert.Equal("https://example.org/entity/42", (string)structured["reference_error"]); Assert.Equal(StructuredError.CurrentVersion, (int)structured["version"]); } @@ -60,14 +54,88 @@ public void PlainException_output_is_backward_compatible() Assert.Single(dict); } + [Fact] + public void Message_is_carried_under_snake_case_message_key() + { + var exception = new StructuredErrorException( + "VALIDATION_FAILED", + "Input field not recognized", + "https://example.org/entity/7", + "Field 'widget_id' is not present in schema 'default'." + ); + + var structured = StructuredErrorOf(SerializeCatchOutput(exception)); + + Assert.Equal("VALIDATION_FAILED", (string)structured["code"]); + Assert.Equal("Input field not recognized", (string)structured["reason"]); + Assert.Equal("Field 'widget_id' is not present in schema 'default'.", (string)structured["message"]); + Assert.Equal("https://example.org/entity/7", (string)structured["reference_error"]); + } + + [Fact] + public void Message_reaches_error_message_and_reason_for_incompletion() + { + // error_message is set from Exception.Message, which the message-taking constructor overrides. The same + // value is what the execution manager sends as TaskResult.ReasonForIncompletion (the Conductor UI banner). + var exception = new StructuredErrorException("CODE", "Short reason", null, "Long diagnostic detail"); + + Assert.Equal("Long diagnostic detail", exception.Message); + Assert.Equal("Long diagnostic detail", (string)SerializeCatchOutput(exception)["error_message"]); + } + + [Fact] + public void Message_is_omitted_when_no_message_was_supplied() + { + // Guards the backward-compatibility promise: pre-existing call sites must keep their exact payload. + var structured = StructuredErrorOf( + SerializeCatchOutput(new StructuredErrorException("CODE", "Short reason", "https://example.org/entity/1")) + ); + + Assert.Null(structured["message"]); + Assert.Equal( + new[] { "code", "reason", "reference_error", "version" }, + ((JObject)structured).Properties().Select(p => p.Name).OrderBy(n => n) + ); + } + + [Fact] + public void Message_is_omitted_when_it_only_repeats_the_reason() + { + var exception = new StructuredErrorException("CODE", "Same text", null, "Same text"); + + Assert.Null(StructuredErrorOf(SerializeCatchOutput(exception))["message"]); + } + + [Fact] + public void Message_survives_the_round_trip() + { + var dict = SerializeCatchOutput( + new StructuredErrorException("CODE", "Short reason", "https://example.org/entity/9", "Long diagnostic detail") + ); + + Assert.True(StructuredErrorSerializer.TryParse(dict, out var parsed)); + Assert.Equal("CODE", parsed.Code); + Assert.Equal("Short reason", parsed.Reason); + Assert.Equal("Long diagnostic detail", parsed.Message); + Assert.Equal("https://example.org/entity/9", parsed.ReferenceError); + } + + [Fact] + public void FromException_returns_null_for_a_plain_exception() + { + Assert.Null(StructuredError.FromException(new System.InvalidOperationException("boom"))); + } + [Fact] public void RoundTrip_helper_output_is_parsed_back() { + // The signal-sender producer: no exception to catch, so the payload is rendered from the model directly. var error = new StructuredError { Code = "UNCLASSIFIED", Reason = "generic failure", - ReferenceError = "https://rom/resourceOrder/7" + Message = "downstream call failed: connection refused", + ReferenceError = "https://example.org/entity/7" }; var outputData = StructuredErrorSerializer.ToOutputData(error); @@ -75,6 +143,7 @@ public void RoundTrip_helper_output_is_parsed_back() Assert.True(StructuredErrorSerializer.TryParse(outputData, out var parsed)); Assert.Equal(error.Code, parsed.Code); Assert.Equal(error.Reason, parsed.Reason); + Assert.Equal(error.Message, parsed.Message); Assert.Equal(error.ReferenceError, parsed.ReferenceError); Assert.Equal(error.Version, parsed.Version); } @@ -123,5 +192,17 @@ public void TryParse_returns_false_when_code_missing() Assert.False(StructuredErrorSerializer.TryParse(dict, out _)); } + + [Fact] + public void TryParse_tolerates_a_message_only_payload_by_degrading() + { + // A message without a code is still unstructured: the caller must fall back to the generic path. + var dict = new Dictionary + { + [StructuredErrorSerializer.OutputKey] = new Dictionary { ["message"] = "detail but no code" } + }; + + Assert.False(StructuredErrorSerializer.TryParse(dict, out _)); + } } }