Refactor param binding failure handling in RequestDelegate#64539
Open
marcominerva wants to merge 2 commits intodotnet:mainfrom
Open
Refactor param binding failure handling in RequestDelegate#64539marcominerva wants to merge 2 commits intodotnet:mainfrom
marcominerva wants to merge 2 commits intodotnet:mainfrom
Conversation
Refactor `RequestDelegateFactory` to handle parameter binding failures more flexibly: - Execute filter pipelines on failure if filters are present and return type is `ValueTask<object?>`, allowing filters to observe/modify the 400 response. - Short-circuit with a 400 response if no filters are present. - Simplify logic using `Expression.Condition` and `Expression.Block`. Add new test cases in `RouteHandlerEndpointRouteBuilderExtensionsTest.cs`: - Validate behavior with/without filters and custom responses. - Ensure handlers are not executed on binding failure.
marcominerva
requested review from
BrennanConroy,
captainsafia and
halter73
as code owners
github-actions
bot
added
the
area-networking
dotnet-policy-service
bot
added
the
community-contribution
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors parameter binding failure handling in RequestDelegateFactory to provide more flexible filter integration. The key change allows endpoint filters to observe and potentially customize 400 Bad Request responses when parameter binding fails, rather than always short-circuiting before filters run.
Key Changes:
- Unified the parameter binding failure logic using
Expression.Conditioninstead of separate code paths for filtered vs non-filtered scenarios - When filters are present and return type is
ValueTask<object?>, the filter pipeline executes on binding failure, allowing filters to customize the 400 response - Added comprehensive test coverage for binding failure scenarios with and without filters
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Http/Http.Extensions/src/RequestDelegateFactory.cs | Refactored CreateParamCheckingResponseWritingMethodCall to conditionally run filter pipelines on parameter binding failures, simplifying the code with a unified Expression.Condition approach |
| src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs | Added 4 test cases validating parameter binding failure behavior: with/without filters, with custom filter responses, and with validation filters |
src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs
Show resolved
Hide resolved
dotnet-policy-service
bot
added
the
pending-ci-rerun
This was referenced Dec 3, 2025
Contributor
|
@marcominerva To sanity check this, can you add tests for the short-circuiting logic in the infrastructure that is shared between RDG/RDF? That'll allow us to confirm that the behavior is the same across runtime and compile-time generated handler implementations. The shared tests are implemented here. |
… between RDG/RDF This confirm that the behavior is the same across runtime and compile-time generated handler implementations.
Contributor
Author
|
@captainsafia, I have added the new tests. |
Contributor
Author
|
I hope this PR will be considered for .NET 11, as I believe it is an important fix. |
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
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.
Description
Refactor
RequestDelegateFactoryto handle parameter binding failures more flexibly:ValueTask<object?>, allowing filters to observe/modify the 400 response.Expression.ConditionandExpression.Block.Add new test cases in
RouteHandlerEndpointRouteBuilderExtensionsTest.cs:Fixes #64341