Skip to content

Commit be4b4ec

Browse files
ognjenkaticclaude
andcommitted
CxODEV-1884: carry a diagnostic message on the structured_error contract
Backport of the 4.2.0 change to the 3.8 line, which consuming services pin. Identical patch: the five touched files were byte-identical between v3.8.0 and master. StructuredErrorException supported only code and reason, so callers that need both a short, stable reason and a detailed explanation had nowhere to put the detail and had to fold it into reason. That defeats matching on reason downstream, and leaves consumers with no separate diagnostic field. Add StructuredError.Message, populated from the exception. No Message property is added to the exception itself -- it already has one by virtue of being an Exception, and the new constructors set it via base(). The mapping only emits message when it differs from reason, so payloads from existing call sites are byte-identical and the field is omitted entirely (NullValueHandling.Ignore), keeping the shape at version 1. message trails referenceError in the new constructors rather than following reason. Overload resolution cannot pick between (code, reason, referenceError, message) and the existing (code, reason, referenceError, innerException) when the fourth argument is an untyped null -- and (code, reason, message, null), a message with no drill-down URI, is the common case. Placing the nullable parameter third leaves only (code, reason, referenceError, null) ambiguous, which the three-argument constructor already expresses. Both execution managers had a hand-copied exception-to-payload mapping and the test mirrored rather than called it, so a new field could pass tests while being silently dropped by the type-poll path. Extract the mapping into StructuredError.FromException and point all three at it. Released as 3.8.1 rather than a 3.8.0-suffixed build: under semver a hyphenated suffix is a pre-release and sorts before 3.8.0, so consumers on 3.8.0 would never be offered it as an upgrade. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent e61f2b8 commit be4b4ec

9 files changed

Lines changed: 176 additions & 44 deletions

File tree

‎src/ConductorSharp.Client/ConductorSharp.Client.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<Authors>Codaxy</Authors>
77
<Company>Codaxy</Company>
88
<PackageId>ConductorSharp.Client</PackageId>
9-
<Version>3.8.0</Version>
9+
<Version>3.8.1</Version>
1010
<Description>Client library for Netflix Conductor, with some additional quality of life features.</Description>
1111
<RepositoryUrl>https://github.com/codaxy/conductor-sharp</RepositoryUrl>
1212
<PackageTags>netflix;conductor</PackageTags>

‎src/ConductorSharp.Engine/ConductorSharp.Engine.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<Authors>Codaxy</Authors>
77
<Company>Codaxy</Company>
88
<PackageId>ConductorSharp.Engine</PackageId>
9-
<Version>3.8.0</Version>
9+
<Version>3.8.1</Version>
1010
<Description>Client library for Netflix Conductor, with some additional quality of life features.</Description>
1111
<RepositoryUrl>https://github.com/codaxy/conductor-sharp</RepositoryUrl>
1212
<PackageTags>netflix;conductor</PackageTags>

‎src/ConductorSharp.Engine/Exceptions/StructuredErrorException.cs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,22 @@ namespace ConductorSharp.Engine.Exceptions
55
/// <summary>
66
/// Thrown by a worker to attach a structured, sanitized error classification to the failed task's output.
77
/// When caught by the execution manager, the <see cref="Code"/>/<see cref="Reason"/>/<see cref="ReferenceError"/>
8-
/// are serialized under the <c>structured_error</c> output key (see
8+
/// and the diagnostic message are serialized under the <c>structured_error</c> output key (see
99
/// <see cref="ConductorSharp.Engine.Util.StructuredErrorSerializer"/>), in addition to the plain
1010
/// <c>error_message</c>, so downstream consumers can read a stable classification without parsing free-text
1111
/// reasons. Plain exceptions are unaffected and keep producing only <c>error_message</c>.
1212
/// </summary>
13+
/// <remarks>
14+
/// There is deliberately no <c>Message</c> property here: the diagnostic message is carried by the inherited
15+
/// <see cref="Exception.Message"/>, which the message-taking constructors set. When no message is supplied it
16+
/// falls back to <see cref="Reason"/>, matching the behaviour of the original constructors.
17+
/// </remarks>
1318
public class StructuredErrorException : Exception
1419
{
1520
/// <summary>Stable, opaque classification code. Consumers map this to a failure response.</summary>
1621
public string Code { get; }
1722

18-
/// <summary>Human-readable, sanitized reason. Safe to surface across a layer boundary.</summary>
23+
/// <summary>Short, stable, sanitized reason. Safe to surface across a layer boundary.</summary>
1924
public string Reason { get; }
2025

2126
/// <summary>Optional URI pointing at the entity where the failure originated (drill-down link).</summary>
@@ -36,5 +41,35 @@ public StructuredErrorException(string code, string reason, string referenceErro
3641
Reason = reason;
3742
ReferenceError = referenceError;
3843
}
44+
45+
/// <summary>
46+
/// Declares a diagnostic <paramref name="message"/> distinct from the short, stable
47+
/// <paramref name="reason"/>. Pass <c>null</c> for <paramref name="message"/> to fall back to the reason,
48+
/// and <c>null</c> for <paramref name="referenceError"/> when there is no entity to drill down into.
49+
/// </summary>
50+
/// <remarks>
51+
/// <paramref name="message"/> trails <paramref name="referenceError"/> rather than following
52+
/// <paramref name="reason"/> (the TMF field order) on purpose. Overload resolution cannot choose between
53+
/// this constructor and the <see cref="Exception"/> one when the fourth argument is an untyped <c>null</c>,
54+
/// so the nullable parameter is placed third, where it is typed the same either way. The only call this
55+
/// leaves ambiguous is <c>(code, reason, referenceError, null)</c> — a declared-but-null inner exception,
56+
/// which the three-argument constructor already expresses. Disambiguate with a named argument if needed.
57+
/// </remarks>
58+
public StructuredErrorException(string code, string reason, string referenceError, string message)
59+
: base(message ?? reason)
60+
{
61+
Code = code;
62+
Reason = reason;
63+
ReferenceError = referenceError;
64+
}
65+
66+
/// <inheritdoc cref="StructuredErrorException(string, string, string, string)"/>
67+
public StructuredErrorException(string code, string reason, string referenceError, string message, Exception innerException)
68+
: base(message ?? reason, innerException)
69+
{
70+
Code = code;
71+
Reason = reason;
72+
ReferenceError = referenceError;
73+
}
3974
}
4075
}

‎src/ConductorSharp.Engine/ExecutionManager.cs‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -247,19 +247,9 @@ await _taskManager.UpdateAsync(
247247
pollResponse.WorkflowInstanceId
248248
);
249249

250-
var errorMessage = new ErrorOutput { ErrorMessage = exception.Message };
251-
252250
// A worker may throw a StructuredErrorException to attach a sanitized, stable classification to the
253251
// failed task's output. Plain exceptions keep producing only error_message, preserving backward compatibility.
254-
if (exception is StructuredErrorException structuredException)
255-
{
256-
errorMessage.StructuredError = new StructuredError
257-
{
258-
Code = structuredException.Code,
259-
Reason = structuredException.Reason,
260-
ReferenceError = structuredException.ReferenceError
261-
};
262-
}
252+
var errorMessage = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) };
263253

264254
// TODO: We should verify that this is alright, it is possible that when executed concurrently,
265255
// the updates caused by LogAsync will be discarded because the call of UpdateAsync(TaskResult...)

‎src/ConductorSharp.Engine/Model/StructuredError.cs‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
using System;
2+
using ConductorSharp.Engine.Exceptions;
3+
14
namespace ConductorSharp.Engine.Model
25
{
36
/// <summary>
@@ -12,13 +15,43 @@ public class StructuredError
1215
/// <summary>Stable, opaque classification code (e.g. an implementation-defined code, or <c>UNCLASSIFIED</c>).</summary>
1316
public string Code { get; set; }
1417

15-
/// <summary>Human-readable, sanitized reason.</summary>
18+
/// <summary>Short, stable, sanitized reason. Consumers may key off this text, so keep it terse.</summary>
1619
public string Reason { get; set; }
1720

21+
/// <summary>
22+
/// Optional diagnostic detail, longer and more specific than <see cref="Reason"/> — the explanation an
23+
/// operator needs, kept out of <see cref="Reason"/> so that stays short and stable. Null when the producer
24+
/// supplied nothing distinct from the reason, in which case it is omitted from serialized output
25+
/// (NullValueHandling.Ignore) and the payload is unchanged from before this field existed.
26+
/// </summary>
27+
public string Message { get; set; }
28+
1829
/// <summary>Optional URI pointing at the entity where the failure originated (drill-down link).</summary>
1930
public string ReferenceError { get; set; }
2031

2132
/// <summary>Payload shape version marker. Defaults to <see cref="CurrentVersion"/>.</summary>
2233
public int Version { get; set; } = CurrentVersion;
34+
35+
/// <summary>
36+
/// Maps a thrown exception onto the payload, returning <c>null</c> for anything that is not a
37+
/// <see cref="StructuredErrorException"/> so plain exceptions keep producing only <c>error_message</c>.
38+
/// This is the single exception-to-payload mapping: both execution managers and the contract tests call it,
39+
/// so the two poll strategies cannot drift apart as the shape evolves.
40+
/// </summary>
41+
public static StructuredError FromException(Exception exception)
42+
{
43+
if (exception is not StructuredErrorException structuredException)
44+
return null;
45+
46+
return new StructuredError
47+
{
48+
Code = structuredException.Code,
49+
Reason = structuredException.Reason,
50+
// Exception.Message falls back to Reason when the thrower supplied no distinct detail, so only
51+
// carry it when it actually adds something. Existing call sites keep their exact payload.
52+
Message = structuredException.Message == structuredException.Reason ? null : structuredException.Message,
53+
ReferenceError = structuredException.ReferenceError
54+
};
55+
}
2356
}
2457
}

‎src/ConductorSharp.Engine/TypePollSpreadingExecutionManager.cs‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -256,19 +256,9 @@ await _taskManager.UpdateAsync(
256256
pollResponse.WorkflowInstanceId
257257
);
258258

259-
var errorMessage = new ErrorOutput { ErrorMessage = exception.Message };
260-
261259
// A worker may throw a StructuredErrorException to attach a sanitized, stable classification to the
262260
// failed task's output. Plain exceptions keep producing only error_message, preserving backward compatibility.
263-
if (exception is StructuredErrorException structuredException)
264-
{
265-
errorMessage.StructuredError = new StructuredError
266-
{
267-
Code = structuredException.Code,
268-
Reason = structuredException.Reason,
269-
ReferenceError = structuredException.ReferenceError
270-
};
271-
}
261+
var errorMessage = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) };
272262

273263
// TODO: We should verify that this is alright, it is possible that when executed concurrently,
274264
// the updates caused by LogAsync will be discarded because the call of UpdateAsync(TaskResult...)

‎src/ConductorSharp.KafkaCancellationNotifier/ConductorSharp.KafkaCancellationNotifier.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
<TargetFramework>net6.0</TargetFramework>
55
<ImplicitUsings>enable</ImplicitUsings>
66
<Nullable>enable</Nullable>
7-
<Version>3.8.0</Version>
7+
<Version>3.8.1</Version>
88
<Authors>Codaxy</Authors>
99
<Company>Codaxy</Company>
1010
</PropertyGroup>

‎src/ConductorSharp.Patterns/ConductorSharp.Patterns.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<GeneratePackageOnBuild>False</GeneratePackageOnBuild>
88
<Authors>Codaxy</Authors>
99
<Company>Codaxy</Company>
10-
<Version>3.8.0</Version>
10+
<Version>3.8.1</Version>
1111
</PropertyGroup>
1212

1313
<ItemGroup>

‎test/ConductorSharp.Engine.Tests/Unit/StructuredErrorTests.cs‎

Lines changed: 99 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Collections.Generic;
2+
using System.Linq;
23
using ConductorSharp.Client;
34
using ConductorSharp.Client.Util;
45
using ConductorSharp.Engine.Exceptions;
@@ -12,26 +13,19 @@ namespace ConductorSharp.Engine.Tests.Unit
1213
{
1314
public class StructuredErrorTests
1415
{
15-
// Mirrors the execution-manager catch block: builds the ErrorOutput (setting StructuredError for a
16-
// StructuredErrorException) and serializes it. TryParse below asserts this output round-trips through the
17-
// shared serializer, pinning the property-derived key/shape to StructuredErrorSerializer.OutputKey.
16+
// Mirrors the execution-manager catch block. It calls StructuredError.FromException — the same mapping both
17+
// ExecutionManager and TypePollSpreadingExecutionManager use — rather than reimplementing it, so a field
18+
// added to the payload cannot pass here while being dropped by one of the managers.
1819
private static IDictionary<string, object> SerializeCatchOutput(System.Exception exception)
1920
{
20-
var output = new ErrorOutput { ErrorMessage = exception.Message };
21-
22-
if (exception is StructuredErrorException structuredException)
23-
{
24-
output.StructuredError = new StructuredError
25-
{
26-
Code = structuredException.Code,
27-
Reason = structuredException.Reason,
28-
ReferenceError = structuredException.ReferenceError
29-
};
30-
}
21+
var output = new ErrorOutput { ErrorMessage = exception.Message, StructuredError = StructuredError.FromException(exception) };
3122

3223
return SerializationHelper.ObjectToDictionary(output, ConductorConstants.IoJsonSerializerSettings);
3324
}
3425

26+
private static JToken StructuredErrorOf(IDictionary<string, object> dict) =>
27+
JObject.Parse(JsonConvert.SerializeObject(dict))[StructuredErrorSerializer.OutputKey];
28+
3529
[Fact]
3630
public void StructuredErrorException_produces_snake_case_structured_error()
3731
{
@@ -42,7 +36,7 @@ public void StructuredErrorException_produces_snake_case_structured_error()
4236
Assert.True(dict.ContainsKey("error_message"));
4337
Assert.True(dict.ContainsKey(StructuredErrorSerializer.OutputKey));
4438

45-
var structured = JObject.Parse(JsonConvert.SerializeObject(dict))["structured_error"];
39+
var structured = StructuredErrorOf(dict);
4640
Assert.Equal("RESOURCE_UNAVAILABLE", (string)structured["code"]);
4741
Assert.Equal("No port available", (string)structured["reason"]);
4842
Assert.Equal("https://rom/resourceOrder/42", (string)structured["reference_error"]);
@@ -60,13 +54,90 @@ public void PlainException_output_is_backward_compatible()
6054
Assert.Single(dict);
6155
}
6256

57+
[Fact]
58+
public void Message_is_carried_under_snake_case_message_key()
59+
{
60+
var exception = new StructuredErrorException(
61+
"ORDER_ITEM_VALIDATION",
62+
"Characteristic not in specification",
63+
"https://som/serviceOrder/7",
64+
"Service characteristic 'tenantId1' not found in service specification 'NaaSSvc-B2B_Internet'."
65+
);
66+
67+
var structured = StructuredErrorOf(SerializeCatchOutput(exception));
68+
69+
Assert.Equal("ORDER_ITEM_VALIDATION", (string)structured["code"]);
70+
Assert.Equal("Characteristic not in specification", (string)structured["reason"]);
71+
Assert.Equal(
72+
"Service characteristic 'tenantId1' not found in service specification 'NaaSSvc-B2B_Internet'.",
73+
(string)structured["message"]
74+
);
75+
Assert.Equal("https://som/serviceOrder/7", (string)structured["reference_error"]);
76+
}
77+
78+
[Fact]
79+
public void Message_reaches_error_message_and_reason_for_incompletion()
80+
{
81+
// error_message is set from Exception.Message, which the message-taking constructor overrides. The same
82+
// value is what the execution manager sends as TaskResult.ReasonForIncompletion (the Conductor UI banner).
83+
var exception = new StructuredErrorException("CODE", "Short reason", null, "Long diagnostic detail");
84+
85+
Assert.Equal("Long diagnostic detail", exception.Message);
86+
Assert.Equal("Long diagnostic detail", (string)SerializeCatchOutput(exception)["error_message"]);
87+
}
88+
89+
[Fact]
90+
public void Message_is_omitted_when_no_message_was_supplied()
91+
{
92+
// Guards the backward-compatibility promise: pre-existing call sites must keep their exact payload.
93+
var structured = StructuredErrorOf(
94+
SerializeCatchOutput(new StructuredErrorException("CODE", "Short reason", "https://rom/resourceOrder/1"))
95+
);
96+
97+
Assert.Null(structured["message"]);
98+
Assert.Equal(
99+
new[] { "code", "reason", "reference_error", "version" },
100+
((JObject)structured).Properties().Select(p => p.Name).OrderBy(n => n)
101+
);
102+
}
103+
104+
[Fact]
105+
public void Message_is_omitted_when_it_only_repeats_the_reason()
106+
{
107+
var exception = new StructuredErrorException("CODE", "Same text", null, "Same text");
108+
109+
Assert.Null(StructuredErrorOf(SerializeCatchOutput(exception))["message"]);
110+
}
111+
112+
[Fact]
113+
public void Message_survives_the_round_trip()
114+
{
115+
var dict = SerializeCatchOutput(
116+
new StructuredErrorException("CODE", "Short reason", "https://rom/resourceOrder/9", "Long diagnostic detail")
117+
);
118+
119+
Assert.True(StructuredErrorSerializer.TryParse(dict, out var parsed));
120+
Assert.Equal("CODE", parsed.Code);
121+
Assert.Equal("Short reason", parsed.Reason);
122+
Assert.Equal("Long diagnostic detail", parsed.Message);
123+
Assert.Equal("https://rom/resourceOrder/9", parsed.ReferenceError);
124+
}
125+
126+
[Fact]
127+
public void FromException_returns_null_for_a_plain_exception()
128+
{
129+
Assert.Null(StructuredError.FromException(new System.InvalidOperationException("boom")));
130+
}
131+
63132
[Fact]
64133
public void RoundTrip_helper_output_is_parsed_back()
65134
{
135+
// The signal-sender producer: no exception to catch, so the payload is rendered from the model directly.
66136
var error = new StructuredError
67137
{
68138
Code = "UNCLASSIFIED",
69139
Reason = "generic failure",
140+
Message = "workflow 1a2b / task 3c4d failed: connection refused",
70141
ReferenceError = "https://rom/resourceOrder/7"
71142
};
72143

@@ -75,6 +146,7 @@ public void RoundTrip_helper_output_is_parsed_back()
75146
Assert.True(StructuredErrorSerializer.TryParse(outputData, out var parsed));
76147
Assert.Equal(error.Code, parsed.Code);
77148
Assert.Equal(error.Reason, parsed.Reason);
149+
Assert.Equal(error.Message, parsed.Message);
78150
Assert.Equal(error.ReferenceError, parsed.ReferenceError);
79151
Assert.Equal(error.Version, parsed.Version);
80152
}
@@ -123,5 +195,17 @@ public void TryParse_returns_false_when_code_missing()
123195

124196
Assert.False(StructuredErrorSerializer.TryParse(dict, out _));
125197
}
198+
199+
[Fact]
200+
public void TryParse_tolerates_a_message_only_payload_by_degrading()
201+
{
202+
// A message without a code is still unstructured: the caller must fall back to the generic path.
203+
var dict = new Dictionary<string, object>
204+
{
205+
[StructuredErrorSerializer.OutputKey] = new Dictionary<string, object> { ["message"] = "detail but no code" }
206+
};
207+
208+
Assert.False(StructuredErrorSerializer.TryParse(dict, out _));
209+
}
126210
}
127211
}

0 commit comments

Comments
 (0)