Give every validation failure one coded, localizable problem shape - #8237
Draft
Hinton wants to merge 1 commit into
Draft
Give every validation failure one coded, localizable problem shape#8237Hinton wants to merge 1 commit into
Hinton wants to merge 1 commit into
Conversation
Hinton
force-pushed
the
validation/coded-problem-responses
branch
from
August 20, 2026 17:09
1380a5b to
2b15be5
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| await result.ExecuteAsync(context); | ||
|
|
||
| stream.Position = 0; | ||
| return Indent(await new StreamReader(stream).ReadToEndAsync()); |
Hinton
force-pushed
the
validation/coded-problem-responses
branch
from
August 20, 2026 18:22
2b15be5 to
4185f12
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8237 +/- ##
==========================================
+ Coverage 63.28% 63.37% +0.09%
==========================================
Files 2401 2407 +6
Lines 104003 104394 +391
Branches 9417 9480 +63
==========================================
+ Hits 65817 66163 +346
- Misses 35930 35955 +25
- Partials 2256 2276 +20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Hinton
force-pushed
the
validation/coded-problem-responses
branch
from
August 21, 2026 08:16
4185f12 to
f027519
Compare
A request that failed its DataAnnotations answered with the ErrorResponseModel envelope while a request its handler rejected answered with an RFC 7807 problem document. Same endpoint, same field, two bodies and two key conventions — a client needed both parsers, and only one of them carried a code it could switch on. One document now serves both. It is keyed by the property as the client sent it rather than the CLR name, so JsonPropertyName is honoured and a nested or indexed path reads the way the request was written. Codes come from ValidationCodes and parameter keys from ValidationParameters, shared lists both sides draw on, because a client looking a substitution up by name is broken by a second spelling as surely as by a second code. A code names what went wrong and never the field it is keyed under. Parameters carry the limit that was breached and nothing derived from the value that breached it. DataAnnotations records a message and discards the constraint behind it: the 200 in StringLength(200) is gone by the time the failure is reported. ValidationCodeMap recovers it. Nothing there validates anything — the framework decides whether a value is valid and this only names what it found — so a path it does not recognise is still reported, uncoded, rather than dropped for want of a name. Recovery happens two ways. Controllers reflect over the request model, because AddControllers declares MVC unsupported under trimming and native AOT and there is nothing there for a generator to save; those entry points say so with RequiresUnreferencedCode rather than leaving a caller to find out after publish. Minimal APIs read a map emitted at build time and never reflect, which is what survives publishing. HttpExtensions builds with IsAotCompatible, so anything new that reflects without declaring it fails the build. A property carrying one constraint is identified by its path alone, so most of the map depends on no wording at all. A property carrying several has its attributes reconstructed and asked — FormatErrorMessage rather than a stored copy, so a framework reword moves both sides together. MaxLength, MinLength and Compare cannot be reconstructed under trimming, but only one fewer candidate than there are needs identifying, so one is left as the fallback. BWVAL001 warns when two on a property are both unaskable and the choice would be a guess. The document is behind CodedValidationProblems. It replaces a body clients are already parsing, so each surface opts in on its own schedule and only the internal API does today; the public API keeps its published shape, which is versioned on its own terms. With the flag off, nothing answers differently.
Hinton
force-pushed
the
validation/coded-problem-responses
branch
from
August 21, 2026 08:22
f027519 to
69cc399
Compare
Comment on lines
+167
to
+170
| catch (Exception) | ||
| { | ||
| return null; | ||
| } |
Comment on lines
+322
to
+328
| foreach (var candidate in type.GetInterfaces().Append(type)) | ||
| { | ||
| if (candidate.IsGenericType && candidate.GetGenericTypeDefinition() == typeof(IEnumerable<>)) | ||
| { | ||
| return candidate.GetGenericArguments()[0]; | ||
| } | ||
| } |
Comment on lines
+181
to
+187
| foreach (var pair in attribute.NamedArguments) | ||
| { | ||
| if (pair.Key == name) | ||
| { | ||
| return pair.Value; | ||
| } | ||
| } |
Comment on lines
+183
to
+190
| foreach (var attribute in property.GetAttributes()) | ||
| { | ||
| if (AttributeTranslator.IsValidationAttribute(attribute) && | ||
| AttributeTranslator.Translate(attribute) is { } translation) | ||
| { | ||
| translations.Add(translation); | ||
| } | ||
| } |
Comment on lines
+221
to
+227
| foreach (var seen in visiting) | ||
| { | ||
| if (SymbolEqualityComparer.Default.Equals(seen, candidate)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
Comment on lines
+255
to
+262
| foreach (var iface in named.AllInterfaces.Concat(new[] { named })) | ||
| { | ||
| if (iface.IsGenericType && | ||
| iface.ConstructedFrom.SpecialType == SpecialType.System_Collections_Generic_IEnumerable_T) | ||
| { | ||
| return (Walkable(iface.TypeArguments[0]), true); | ||
| } | ||
| } |
Comment on lines
+293
to
+301
| foreach (var attribute in property.GetAttributes()) | ||
| { | ||
| if (attribute.AttributeClass?.ToDisplayString() == "System.Text.Json.Serialization.JsonPropertyNameAttribute" && | ||
| attribute.ConstructorArguments.Length > 0 && | ||
| attribute.ConstructorArguments[0].Value is string name) | ||
| { | ||
| return name; | ||
| } | ||
| } |
Comment on lines
+317
to
+323
| foreach (var named in attribute.NamedArguments) | ||
| { | ||
| if (named.Key == "Name" && named.Value.Value is string display) | ||
| { | ||
| return display; | ||
| } | ||
| } |
Comment on lines
+47
to
+64
| foreach (var modelError in entry.Errors) | ||
| { | ||
| var message = Describe(modelError); | ||
|
|
||
| if (!TryResolveAny(rootTypes, key, message, out var wirePath, out var error)) | ||
| { | ||
| wirePath = ToWireName(key); | ||
| error = new ErrorCode(ValidationCodes.Invalid, message); | ||
| } | ||
|
|
||
| if (!errors.TryGetValue(wirePath, out var codes)) | ||
| { | ||
| codes = []; | ||
| errors[wirePath] = codes; | ||
| } | ||
|
|
||
| codes.Add(error); | ||
| } |
Member
Author
|
Alternative in #8242: same wire contract and feature flag, but codes recovered by recognising the framework's message rather than by a generated map. No source generator, no reflection, no Trade is that the wording becomes the contract: an explicit |
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
1 participant
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.
🎟️ Tracking
No ticket — foundation extracted from the PAM error-codes work so it can land on
mainindependently of it.📔 Objective
A request that failed its DataAnnotations answered with the
ErrorResponseModelenvelope while a request its handler rejected answered with an RFC 7807 problem document. Same endpoint, same field, two bodies and two key conventions — a client needed both parsers, and only one of them carried a code it could switch on.One document now serves both:
{ "type": "validation_error", "title": "One or more validation errors occurred.", "status": 400, "errors": { "reason": [{ "type": "required", "detail": "Reason is required." }], "name": [{ "type": "too_long", "detail": "Name must be 200 characters or shorter.", "parameters": { "max": 200 } }], "members[1].email": [{ "type": "invalid_email", "detail": "Email is not an address." }] } }Keyed the way the client wrote it.
[JsonPropertyName]is honoured, nesting and collection indices are preserved. Model state keys the CLR name, which today leaks straight throughErrorResponseModel; a property renamed on the wire is currently reported under a name the client never sent.One vocabulary. Codes come from
ValidationCodesand parameter keys fromValidationParameters. A client that looks a substitution up by name is broken by a second spelling as surely as by a second code. A code names what went wrong and never the field it is keyed under —required, notname_required. Parameters carry the limit that was breached and nothing derived from the value that breached it: a length ceiling, never the string that overran it.Codes recovered, not reimplemented. DataAnnotations records a message and discards the constraint behind it — the
200in[StringLength(200)]is gone by the time the failure is reported.ValidationCodeMaprecovers it. Nothing in this PR validates anything; the framework still decides whether a value is valid and this only names what it found. A path it does not recognise is still reported, asinvalidwith its original message, because a 400 with an emptyerrorsmap tells a client less than nothing.Two resolution paths, and why
ModelStateAddValidation()AddControllers()is annotated[RequiresUnreferencedCode("MVC does not currently support trimming or native AOT.")], so a generated map for the controller surface would make an un-publishable path look publishable. Controllers reflect and the entry points declare it with[RequiresUnreferencedCode]; minimal APIs read the generated map and never reflect.HttpExtensionsbuilds withIsAotCompatible, so anything new that reflects without saying so fails the build — that is what keeps AOT reachable as endpoints migrate off MVC.How a failure is attributed to a constraint
A property with one constraint is identified by its path alone — no message involved, so most of the map depends on no framework wording at all. A property with several has its attributes reconstructed and asked:
Asking beats storing a copy: a framework reword moves both sides together.
MaxLengthAttribute,MinLengthAttributeandCompareAttributehave[RequiresUnreferencedCode]constructors and cannot be reconstructed, but only n − 1 candidates need identifying, so one is left as the fallback.BWVAL001warns when two on a property are both unaskable and the choice would be a guess.[StringLength(n, MinimumLength = m)]reports oneinvalid_lengthcarrying both bounds rather thantoo_long/too_short: the attribute emits an identical message either way, so the direction is genuinely not recoverable. The client composes the sentence from the bounds.Notes for review
FeatureFlagKeys.CodedValidationProblems; off means nothing changes. It replaces a body clients already parse, so each surface opts in separately —TryCodedProblemoffers it and only the internal Api takes it. The public API keeps its published shape, versioned on its own terms.src/HttpExtensions/README.mddocuments the whole thing, including the known gaps below.ValidationProblemDocumentTestsasserts the complete serialized body against a literal, so the wire format has a spec that fails on any shape change.ValidationRoundTripTestsdrives a real MVC pipeline and asserts on codes, so a framework reword fails the build on the next SDK bump rather than silently mis-coding in production.GeneratedCodeMapTestscovers the generated path the same way.Known gaps, deliberately not in scope
ExceptionHandlerFilterAttributestill answers withErrorResponseModel, sothrow new BadRequestException(modelState)bypasses this. Closing that is the natural follow-up.Validator.TryValidateObject, which is what blocks AOT there today. The facility is tested but unconsumed..Produces<BitwardenValidationProblemDetails>(400)is not declared on endpoints, so OpenAPI does not yet describe the coded shape.📸 Screenshots
N/A