fix(http): emit api-versioning headers on non-2xx responses#2661
Closed
outofrange-consulting wants to merge 5 commits intoJasperFx:mainfrom
Closed
fix(http): emit api-versioning headers on non-2xx responses#2661outofrange-consulting wants to merge 5 commits intoJasperFx:mainfrom
outofrange-consulting wants to merge 5 commits intoJasperFx:mainfrom
Conversation
added 3 commits
May 1, 2026 21:41
ApiVersionHeaderWriter now registers headers via Response.OnStarting from the very first frame of every relevant chain, so versioning headers (Deprecation, Sunset, Link, api-supported-versions) are emitted on all framework-produced responses regardless of status code: 2xx, IResult-returning short-circuits, validation ProblemDetails, and middleware-IResult exits. Previously the writer ran as a Postprocessor, which Wolverine codegen skips when MaybeEndWithResultFrame or MaybeEndWithProblemDetailsFrame returns out of the generated handler — leaving 4xx error paths without the headers. A new ApiVersionHeaderFinalizationPolicy is registered in MapWolverineEndpoints after configure(); it runs last and re-positions the writer call to chain.Middleware index 0, outranking FluentValidation / DataAnnotations / RequestId / TenantId frames that also insert at index 0. Out of scope: responses produced by the global ASP.NET Core exception handler bypass the chain pipeline; users wanting headers on 5xx wire them via separate middleware (documented in versioning.md). Tests: 4 new integration tests in api_versioning_error_path_header_tests exercise the four exit paths (404 IResult, 400 validation, 401 middleware short-circuit, success). Sample endpoints added in WolverineWebApi/ApiVersioning/OrdersV1ErrorPathsEndpoint.cs.
- ApiVersionHeaderFinalizationPolicy: switch to IReadOnlySet for O(1) Contains, drop the unreachable re-position branch in favor of a Debug.Assert idempotency invariant. - ApiVersionHeaderWriter: drop per-request tuple boxing in OnStarting by re-fetching endpoint metadata + writer from RequestServices in a static callback; expose WriteVersioningHeadersTo as a public helper so exception-path middleware can reuse the same RFC formatting. - ApiVersioningPolicy: rename WireHeaderPostprocessors to AttachHeaderState, RequiresHeaderWriter to RequiresHeaderEmission, _headerProcessedChains to _headerStateChains. - WolverineWebApi: split OrdersV1ErrorPathsEndpoint.cs into one file per type; add OrdersV1ThrowsEndpoint plus a scoped UseExceptionHandler on /v1/orders/throws to back the new regression test. - api_versioning_error_path_header_tests: pin the documented out-of-scope (5xx via global exception handler emits no versioning headers) and tighten api-supported-versions assertions to the exact expected value rather than ContainsKey. - versioning.md: tighten the 5xx middleware snippet to delegate header emission to ApiVersionHeaderWriter.WriteVersioningHeadersTo so the source of truth lives in one place.
…eader path Second-pass review fixes for PR #3. Production - Rename ApiVersioningPolicy.ChainsRequiringHeaderWriter to ChainsRequiringHeaderEmission to match the rest of the rename pass ("header state / emission" terminology). Property was already internal, so callers in WolverineHttpEndpointRouteBuilderExtensions are updated in lockstep. - ApiVersionHeaderWriter.WriteAsync now resolves the writer via GetRequiredService<T>() inside the OnStarting callback. The previous GetService(typeof(...)) silently swallowed null and made lost headers invisible. Bootstrap registers the writer unconditionally as a singleton, so a missing registration is a programmer error and must fail fast. - Add inline comments to the early-exit gate and the in-OnStarting re-fetch explaining why the writer must re-resolve the endpoint at flush time (middleware can re-route the request between frames). - Mark WriteVersioningHeadersTo with EditorBrowsable(Advanced) and add a class-level XML doc explaining the asymmetry between WriteAsync (chain frame, locked by codegen) and the sync helper for advanced/middleware use. - WolverineWebApi/Program.cs: pin placement intent of UseExceptionHandler with a "before MapWolverineEndpoints" comment. - OrdersV1ThrowsEndpoint: throw message now identifies the source and flags it as IGNORE for log tailers. Tests - New ApiVersionHeaderWriterTests.missing_writer_in_request_services_throws pins the new fail-fast contract for the GetRequiredService change. - New DEBUG-only ApiVersioningPolicyHeaderWiringTests.finalization_assert_- fires_when_writer_was_displaced exercises the Debug.Assert invariant in ApiVersionHeaderFinalizationPolicy via a throwing TraceListener. Guarded by #if DEBUG since the assert is no-op under RELEASE. - api_versioning_error_path_header_tests: - Extract ExpectedSupportedVersions constant for "1.0, 3.0" with a pointer to Program.cs:303-306 so config drift produces a clear single failure rather than four near-identical ones. - Assert response body contains "global-exception-handler" on the 5xx test so a future change letting Wolverine answer the throws endpoint with its own 500 cannot turn the absent-headers assertion into a tautology. Docs - versioning.md exception-handler snippet: add the missing Microsoft.Extensions.DependencyInjection using and switch to GetRequiredService<T>() to match the production change.
Member
|
@outofrange-consulting I'm not good with this one doing the service location. Is there any way you could try to make this a bit more idiomatic Wolverine and eliminate the service location hooks in the hot path? Sorry man, but I don't want to have to support this one. |
Replace the static OnStarting lambda + RequestServices.GetRequiredService re-resolution with a closure that captures `this` and `context`. The writer is already resolved from DI by Wolverine's generated handler (MethodCall.For), so no further service location is needed in the hot path. Trades the static-lambda micro-optimization for one closure allocation per request — the same cost ASP.NET Core middleware pays for OnStarting in general, and the price of being idiomatic. Removes the now-unreachable missing-writer-in-RequestServices fail-fast test.
Author
|
@jeremydmiller addressed — dropped the service-locator hop in |
…n-headers-on-errors # Conflicts: # src/Http/Wolverine.Http/ApiVersioning/ApiVersioningPolicy.cs
Member
|
@outofrange-consulting Hey, can you just combine the two PRs in one rebased PR please? No hurry on my part though |
Author
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2633.
Summary
ApiVersionHeaderWriternow registers headers viaResponse.OnStartingfrom the first frame of every relevant chain — versioning headers (Deprecation,Sunset,Link,api-supported-versions) are emitted on all framework-produced responses regardless of status code (2xx, IResult short-circuits, validationProblemDetails, middleware-IResult exits).Previously emitted as a
Postprocessor, which Wolverine codegen skips whenMaybeEndWithResultFrame/MaybeEndWithProblemDetailsFramereturns out of the generated handler — leaving 4xx error paths without headers.Implementation
ApiVersionHeaderFinalizationPolicyregistered inMapWolverineEndpointsafterconfigure(). Runs last, positions the writer call atchain.Middleware[0](outranksFluentValidation/DataAnnotations/RequestId/TenantIdframes that also insert at index 0). Idempotent: skips when writer is already at index 0;Debug.Assertinvariant under DEBUG.ApiVersionHeaderWriter.WriteAsyncis the chain frame; usesstaticlambda +stateargument to avoid per-request closure allocation. Re-fetchesApiVersionEndpointHeaderStatefrom the matched endpoint inside theOnStartingcallback because the endpoint can be re-routed by middleware between the chain frame and header-flush time. Resolves itself viaRequestServices.GetRequiredService<ApiVersionHeaderWriter>()— fail-fast if DI is misconfigured.ApiVersionHeaderWriter.WriteVersioningHeadersTo(HttpContext, ApiVersionEndpointHeaderState)is exposed aspublic([EditorBrowsable(Advanced)]) so the documented 5xx-middleware snippet can call the same logic without copy-pasting RFC-9745 emission.ChainsRequiringHeaderEmission(IReadOnlySet<HttpChain>) gives the finalization policy O(1) lookup.Out of scope
Responses produced by the global ASP.NET Core exception handler bypass the chain pipeline entirely. Users wanting headers on 5xx wire them via separate middleware. Documented in
versioning.mdwith a snippet that callsWriteVersioningHeadersTo.Test plan
api_versioning_error_path_header_tests— 5 exit-path tests:ProblemDetailsUseExceptionHandler— headers ABSENT (regression test pinning the documented out-of-scope, asserts response body marker"global-exception-handler")ApiVersionHeaderWriterTests— added missing-writer-in-DI fail-fast testApiVersioningPolicyHeaderWiringTests— finalization idempotency + DEBUG-onlyfinalization_assert_fires_when_writer_was_displacedWolverine.Http.Testssuite green: 714/714Sample app
Four split fixtures in
WolverineWebApi/ApiVersioning/:OrdersV1NotFoundEndpoint.csOrdersV1CreateEndpoint.csOrdersV1RestrictedEndpoint.csOrdersV1ThrowsEndpoint.cs(intentional throw, scoped underUseWhen(.../v1/orders/throws, UseExceptionHandler))