From 8ec5fc62e764bafa637eef7b551aed25c6a2532d Mon Sep 17 00:00:00 2001 From: Lewis Renfrew Date: Mon, 5 Oct 2026 12:22:39 +0100 Subject: [PATCH 1/2] ServerFailureResult carries its message in the 500's body; 3.3.0 Like BadRequestResult, UnauthorisedResult and ForbiddenResult - e.g. Linn.Common.Facade 13.6's "The change was saved, but ... failed", so a caller knows not to simply retry. Empty body when there's no message, as before. --- CHANGELOG.md | 5 +++ src/Handlers/ResultResponseWriter.cs | 8 +++- src/Linn.Common.Service.csproj | 2 +- tests/WhenWritingAServerFailure.cs | 64 ++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests/WhenWritingAServerFailure.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 50a1f36..06a4121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ # Changelog +## [3.3.0] - 2026-10-05 +### Changes +- A `ServerFailureResult` (500) now carries its message in the body, like `BadRequestResult`, + `UnauthorisedResult` and `ForbiddenResult` - e.g. Linn.Common.Facade 13.6's "The change was saved, but + ... failed", which tells a caller not to simply retry. Still an empty body when there's no message. ## [3.2.0] - 2026-08-20 ### Changes - StreamCopyingResultHandler now handles ForbiddenResult (403). Previously a forbidden result fell through to the default case and returned 500. diff --git a/src/Handlers/ResultResponseWriter.cs b/src/Handlers/ResultResponseWriter.cs index 08a5edb..a2d0184 100644 --- a/src/Handlers/ResultResponseWriter.cs +++ b/src/Handlers/ResultResponseWriter.cs @@ -77,8 +77,12 @@ await res.WriteAsync( cancellationToken); break; - case ServerFailureResult _: + case ServerFailureResult r: res.StatusCode = 500; + + // e.g. Linn.Common.Facade's "The change was saved, but ... failed" - what a caller needs + // to know not to retry; nothing is written when there's no message + await res.WriteAsync(this.SerializeOptional(NullIfEmpty(r.Message), null), cancellationToken); break; default: @@ -87,6 +91,8 @@ await res.WriteAsync( } } + private static string? NullIfEmpty(string? message) => string.IsNullOrEmpty(message) ? null : message; + private string SerializeOptional(object? a, object? b) { if (a != null) diff --git a/src/Linn.Common.Service.csproj b/src/Linn.Common.Service.csproj index 90411ea..d1103c8 100644 --- a/src/Linn.Common.Service.csproj +++ b/src/Linn.Common.Service.csproj @@ -5,7 +5,7 @@ enable Linn.Common.Service Linn.Common.Service - 3.2.0 + 3.3.0 enable diff --git a/tests/WhenWritingAServerFailure.cs b/tests/WhenWritingAServerFailure.cs new file mode 100644 index 0000000..c8a833c --- /dev/null +++ b/tests/WhenWritingAServerFailure.cs @@ -0,0 +1,64 @@ +namespace Linn.Common.Service.Tests +{ + using System.IO; + using System.Net; + using System.Threading; + using System.Threading.Tasks; + + using FluentAssertions; + + using Linn.Common.Facade; + using Linn.Common.Service.Handlers; + using Linn.Common.Service.Tests.Fake.Resources; + + using Microsoft.AspNetCore.Http; + + using NUnit.Framework; + + public class WhenWritingAServerFailure + { + private JsonResultHandler handler; + + private DefaultHttpContext context; + + [SetUp] + public void SetUp() + { + this.handler = new JsonResultHandler(); + this.context = new DefaultHttpContext(); + this.context.Response.Body = new MemoryStream(); + } + + [Test] + public async Task ShouldSendItsMessage() + { + await this.handler.Handle( + this.context.Request, + this.context.Response, + new ServerFailureResult("The change was saved, but writing its log failed"), + CancellationToken.None); + + this.context.Response.StatusCode.Should().Be((int)HttpStatusCode.InternalServerError); + (await this.Body()).Should().Be("\"The change was saved, but writing its log failed\""); + } + + [Test] + public async Task ShouldSendNothingWithoutAMessage() + { + await this.handler.Handle( + this.context.Request, + this.context.Response, + new ServerFailureResult(), + CancellationToken.None); + + this.context.Response.StatusCode.Should().Be((int)HttpStatusCode.InternalServerError); + (await this.Body()).Should().BeEmpty(); + } + + private async Task Body() + { + this.context.Response.Body.Position = 0; + return await new StreamReader(this.context.Response.Body).ReadToEndAsync(); + } + } +} From 96f4d6f13e6c8dab2ff86ca9df0e2d8cca6ae001 Mon Sep 17 00:00:00 2001 From: Lewis Renfrew Date: Mon, 5 Oct 2026 12:30:06 +0100 Subject: [PATCH 2/2] Send only a ServerFailureResult's UserMessage Message is diagnostic detail and stays unsent, so existing ServerFailureResults (e.g. carrying an upstream service's raw error) are still an empty 500. Needs Linn.Common.Facade 13.6.0. --- CHANGELOG.md | 7 ++++--- src/Handlers/ResultResponseWriter.cs | 6 +++--- src/Linn.Common.Service.csproj | 2 +- tests/Linn.Common.Service.Tests.csproj | 2 +- tests/WhenWritingAServerFailure.cs | 22 ++++++++++------------ 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a4121..196e4ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ # Changelog ## [3.3.0] - 2026-10-05 ### Changes -- A `ServerFailureResult` (500) now carries its message in the body, like `BadRequestResult`, - `UnauthorisedResult` and `ForbiddenResult` - e.g. Linn.Common.Facade 13.6's "The change was saved, but - ... failed", which tells a caller not to simply retry. Still an empty body when there's no message. +- A `ServerFailureResult` (500) now sends its `UserMessage` (new in Linn.Common.Facade 13.6.0, which this + version needs) in the body - text written to be shown, e.g. "The change was saved, but ... failed", + which tells a caller not to simply retry. Its `Message` (diagnostic detail) is never sent, so existing + `ServerFailureResult`s - which have no `UserMessage` - are still an empty 500. ## [3.2.0] - 2026-08-20 ### Changes - StreamCopyingResultHandler now handles ForbiddenResult (403). Previously a forbidden result fell through to the default case and returned 500. diff --git a/src/Handlers/ResultResponseWriter.cs b/src/Handlers/ResultResponseWriter.cs index a2d0184..f249726 100644 --- a/src/Handlers/ResultResponseWriter.cs +++ b/src/Handlers/ResultResponseWriter.cs @@ -80,9 +80,9 @@ await res.WriteAsync( case ServerFailureResult r: res.StatusCode = 500; - // e.g. Linn.Common.Facade's "The change was saved, but ... failed" - what a caller needs - // to know not to retry; nothing is written when there's no message - await res.WriteAsync(this.SerializeOptional(NullIfEmpty(r.Message), null), cancellationToken); + // only the UserMessage, written to be shown (e.g. Linn.Common.Facade's "The change was + // saved, but ... failed"); Message is diagnostic detail and is never sent + await res.WriteAsync(this.SerializeOptional(NullIfEmpty(r.UserMessage), null), cancellationToken); break; default: diff --git a/src/Linn.Common.Service.csproj b/src/Linn.Common.Service.csproj index d1103c8..1fa46f0 100644 --- a/src/Linn.Common.Service.csproj +++ b/src/Linn.Common.Service.csproj @@ -13,7 +13,7 @@ - + diff --git a/tests/Linn.Common.Service.Tests.csproj b/tests/Linn.Common.Service.Tests.csproj index fd2e32b..ebf03d8 100644 --- a/tests/Linn.Common.Service.Tests.csproj +++ b/tests/Linn.Common.Service.Tests.csproj @@ -12,7 +12,7 @@ - + diff --git a/tests/WhenWritingAServerFailure.cs b/tests/WhenWritingAServerFailure.cs index c8a833c..35f4df6 100644 --- a/tests/WhenWritingAServerFailure.cs +++ b/tests/WhenWritingAServerFailure.cs @@ -30,31 +30,29 @@ public void SetUp() } [Test] - public async Task ShouldSendItsMessage() + public async Task ShouldSendTheUserMessageButNotTheDetail() { - await this.handler.Handle( - this.context.Request, - this.context.Response, - new ServerFailureResult("The change was saved, but writing its log failed"), - CancellationToken.None); + await this.Write(new ServerFailureResult( + "The change was saved, but writing its log failed (SqlException: deadlock on log_table)", + "The change was saved, but writing its log failed")); this.context.Response.StatusCode.Should().Be((int)HttpStatusCode.InternalServerError); (await this.Body()).Should().Be("\"The change was saved, but writing its log failed\""); } [Test] - public async Task ShouldSendNothingWithoutAMessage() + public async Task ShouldSendNothingForAnExistingServerFailure() { - await this.handler.Handle( - this.context.Request, - this.context.Response, - new ServerFailureResult(), - CancellationToken.None); + // a message only - diagnostic, e.g. an upstream service's raw error - stays unsent + await this.Write(new ServerFailureResult("Unexpected status code 502: upstream stack trace")); this.context.Response.StatusCode.Should().Be((int)HttpStatusCode.InternalServerError); (await this.Body()).Should().BeEmpty(); } + private Task Write(IResult result) => + this.handler.Handle(this.context.Request, this.context.Response, result, CancellationToken.None); + private async Task Body() { this.context.Response.Body.Position = 0;