Repository navigation
[DeduplicatedWithResponse] replays ASP.NET Core IResult responses - #5352
Conversation
An endpoint that returns an IResult was refused at startup, because its writer is IResult.ExecuteAsync rather than WriteJsonFrame. CompleteDeduplicatedRequest runs before that call, so the response is now described from the result itself: Results<...> unions are unwrapped, the status comes from IStatusCodeHttpResult, the body is the IValueHttpResult value serialized as ASP.NET Core's HttpResultsHelper writes it (its own JsonOptions, the declared TValue unless the runtime type differs and TValue has no known polymorphism), and the Location comes from Created and Accepted. Only Ok, Created, Accepted (with or without a value), NoContent, StatusCodeHttpResult and EmptyHttpResult are stored. A status of 400 or above, a challenge or a forbid gives the claim back as any failure does. Any other result is done work whose response cannot be stored: the claim is kept unanswered, so repeats get 409, and an error names the result type. An endpoint declared to return only results that can never be stored is refused at startup, from the same classification.
Created<T>, Ok<T> with a derived value, NoContent and both arms of a Results<Created<T>, NotFound> union on claim-and-release; a text result that is kept unanswered; the same union claimed inside the Marten transaction; and the startup refusal of a file result next to an accepted union.
- Replay a Location the endpoint set on the response when the result has none of its own - Refuse at startup an IResult interface that no replayable result implements - Recognize the IResult writer by the resource type rather than the writer frame's shape - Resolve the logger only on the error paths - Name every replayable result in the startup refusal - Test Accepted with a Location, and keep the union's failure arm from doing work in the Marten test
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a995dc2da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…nly Results unions - An endpoint declared as object that returns a typed result is written as JSON by the resource writer, so its response is stored the same way rather than as the result's status and value - Only ASP.NET Core's Results<...> unions are unwrapped. A custom INestedHttpResult may write more than the result it holds, so it is refused at startup when declared and kept unanswered when returned
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080d5e09ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A custom IResourceWriterPolicy runs ahead of ResultWriterPolicy and may write an IResult some other way, so the JSON-only check looks for the IResult.ExecuteAsync writer again rather than the resource type
Follow-up to #4742.
[DeduplicatedWithResponse]refuses endpoints that return an ASP.NET CoreIResult, so anything written withTypedResultscan't use it. With this change,Ok,Created,Accepted(with or without a value),NoContent,StatusCodeHttpResultandResults.Emptyare stored and replayed with their status, JSON body andLocation, in both claim-and-release andClaimInTransaction. AResults<...>union replays by the arm it answered with.Start with
CompleteDeduplicatedRequestinHttpHandler.cs, then the startup check inHttpChain.DeduplicatedWithResponse.cs.Behavior
JsonHttpResult,CreatedAtRoute) aren't stored. The claim is kept, repeats get 409, and an error is logged.IResultor a union with a replayable arm is decided per request.Design
Following the #4742 review and what shipped: the stored record stays JSON-only (
DeduplicatedResponse(StatusCode, Body, Location)), declaration defects fail at startup, and no public API changes.ExecuteAsync, because that runs after the commit and both claim modes record before it.ServerSentEventsResult<T>only exposes a status, andCreatedAtRoutecomputesLocationwhile executing.JsonOptionsand the same type rule asHttpResultsHelper.WriteResultAsJsonAsync, so a replay matches the original bytes.Known difference:
Ok<ProblemDetails>replays asapplication/jsonrather thanapplication/problem+json. I left it, since problem details in a success response is not a path endorsed by yours truly.Tests
New cases in
deduplicated_with_response,deduplicated_with_response_declarationand the Marten suite cover each replayed shape, union success and failure arms, a non-replayable text result, startup refusals, andClaimInTransaction. All 129*deduplicat*tests pass.