feat(inbound-api): method-script exception detail on the inbound audit row, never on the HTTP body (D3)

This commit is contained in:
Joseph Doherty
2026-09-09 12:15:00 -04:00
parent a45857ddd6
commit 98c6809a66
6 changed files with 188 additions and 4 deletions
@@ -432,7 +432,23 @@ public sealed class EndpointExtensionsTests : IDisposable
var method = SeedMethod(1, "boom",
"""throw new System.InvalidOperationException("boom-msg");""");
using var host = await BuildHostAsync(method);
// Flow Debugger step logs (design 2026-09-09 D3): the detail the body must NOT carry
// is nonetheless stashed for AuditWriteMiddleware, so the inbound audit row (and the
// Flow Debugger's inbound node) can say why the run failed. A capture middleware reads
// the slot once the endpoint has finished, proving both halves in one request.
string? capturedDetail = null;
using var host = await BuildHostAsync(method, customize: builder =>
{
builder.Use(async (ctx, next) =>
{
await next();
if (ctx.Items.TryGetValue(
AuditWriteMiddleware.ErrorDetailItemKey, out var stashed))
{
capturedDetail = stashed as string;
}
});
});
var token = await SeedKeyAsync(host, "key1", "caller", new[] { "boom" });
var client = host.GetTestClient();
@@ -445,6 +461,10 @@ public sealed class EndpointExtensionsTests : IDisposable
// message is not leaked (the executor sanitises before this point).
Assert.Contains("error", body);
Assert.DoesNotContain("boom-msg", body);
// …while the audit-only slot carries the real cause.
Assert.NotNull(capturedDetail);
Assert.Contains("boom-msg", capturedDetail);
}
[Fact]
@@ -202,6 +202,56 @@ public class InboundScriptExecutorTests
Assert.Equal("Internal script error", result.ErrorMessage);
}
[Fact]
public async Task HandlerThrows_ErrorDetailCarriesTheException_WhileTheMessageStaysSafe()
{
// Flow Debugger step logs (design 2026-09-09 D3): the generic SCRIPT_ERROR catch keeps
// the caller-facing message sanitised, but the exception itself must survive on the
// additive ErrorDetail so the endpoint can put it on the inbound audit row (and the
// Flow Debugger's inbound node can show why the run failed).
var method = new ApiMethod("failing-detail", "throw new InvalidOperationException(\"real reason\");")
{ Id = 1, TimeoutSeconds = 10 };
_executor.RegisterHandler("failing-detail", _ => throw new InvalidOperationException("real reason"));
var result = await _executor.ExecuteAsync(
method,
new Dictionary<string, object?>(),
_route,
TimeSpan.FromSeconds(10));
Assert.False(result.Success);
Assert.Equal("Internal script error", result.ErrorMessage);
Assert.Equal("SCRIPT_ERROR", result.ErrorCode);
Assert.NotNull(result.ErrorDetail);
Assert.Contains("InvalidOperationException", result.ErrorDetail);
Assert.Contains("real reason", result.ErrorDetail);
// The caller-safe fields must never carry it.
Assert.DoesNotContain("real reason", result.ErrorMessage!);
}
[Fact]
public async Task HandlerTimesOut_CarriesNoErrorDetail()
{
// A timeout is fully described by its message + TIMEOUT code — the
// OperationCanceledException adds nothing, so the row stays detail-less.
var method = new ApiMethod("slow-detail", "Thread.Sleep(60000);") { Id = 1, TimeoutSeconds = 1 };
_executor.RegisterHandler("slow-detail", async ctx =>
{
await Task.Delay(TimeSpan.FromSeconds(60), ctx.CancellationToken);
return "never";
});
var result = await _executor.ExecuteAsync(
method,
new Dictionary<string, object?>(),
_route,
TimeSpan.FromMilliseconds(100));
Assert.False(result.Success);
Assert.Equal("TIMEOUT", result.ErrorCode);
Assert.Null(result.ErrorDetail);
}
[Fact]
public async Task RoutedAskTimeout_Returns500WithSiteUnreachableCode()
{
@@ -265,6 +265,74 @@ public class AuditWriteMiddlewareTests
Assert.Equal("kaboom", evt.ErrorMessage);
}
// ---------------------------------------------------------------------
// 4b. Error detail — the endpoint stashes a method script's exception on
// HttpContext.Items (the HTTP body only ever carries the sanitised
// message), and the middleware lifts it onto the row.
// ---------------------------------------------------------------------
[Fact]
public async Task ScriptErrorDetail_StashedOnItems_LandsOnTheRow()
{
var writer = new RecordingAuditWriter();
var ctx = BuildContext();
const string detail = "System.InvalidOperationException: real reason\n at Script.<Main>";
var mw = CreateMiddleware(hc =>
{
hc.Items[AuditWriteMiddleware.ErrorDetailItemKey] = detail;
hc.Response.StatusCode = 500;
return Task.CompletedTask;
}, writer);
await mw.InvokeAsync(ctx);
var evt = Assert.Single(writer.Events);
Assert.Equal(AuditStatus.Failed, evt.Status);
Assert.Equal(detail, evt.ErrorDetail);
}
[Fact]
public async Task NoErrorDetailStashed_LeavesErrorDetailNull()
{
var writer = new RecordingAuditWriter();
var ctx = BuildContext();
var mw = CreateMiddleware(hc =>
{
hc.Response.StatusCode = 200;
return Task.CompletedTask;
}, writer);
await mw.InvokeAsync(ctx);
var evt = Assert.Single(writer.Events);
Assert.Null(evt.ErrorDetail);
}
[Fact]
public async Task ThrownException_ErrorDetail_WinsOverTheStashedItem()
{
// An exception that ESCAPED the pipeline is the richer, more authoritative
// source: it describes the failure the framework error path is about to report.
var writer = new RecordingAuditWriter();
var ctx = BuildContext();
var boom = new InvalidOperationException("kaboom");
var mw = CreateMiddleware(hc =>
{
hc.Items[AuditWriteMiddleware.ErrorDetailItemKey] = "stashed detail";
throw boom;
}, writer);
await Assert.ThrowsAsync<InvalidOperationException>(() => mw.InvokeAsync(ctx));
var evt = Assert.Single(writer.Events);
// Compared by content, not by an exact ToString(): the re-throw keeps appending
// stack frames after the row was built, so the captured text is a prefix of what
// the exception reads as by the time the assertion runs.
Assert.NotNull(evt.ErrorDetail);
Assert.StartsWith(boom.GetType().FullName + ": kaboom", evt.ErrorDetail);
Assert.DoesNotContain("stashed detail", evt.ErrorDetail);
}
// ---------------------------------------------------------------------
// 5. Actor resolution — the endpoint handler stashes the API key name
// AFTER successful auth so the middleware can pick it up from