Separate type/parsing validation error handling from business logic validation (400 vs 422)
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
We'd like our API to be able to return 400 errors for problems where the inbound payload is _syntactically_ incorrect (e.g., a Boolean is provided where we expected a number, or the inbound body can't be parsed into valid JSON); and 422 errors for _semantic_ errors (e.g., required field missing, or a value is outside an acceptable range). This doesn't seem possible without replacing large sections of the validation framework at the moment.
Related:
- dotnet/aspnetcore#6145
- aspnet/Mvc#6789
In previous issues there was never really a resolution to this.
dotnet/aspnetcore#6145 somewhat led to the desire to remove the hardcoded status code, a PR that was never accepted, but there still wasn't a facility to differentiate between error types.
The "HTTP Semantics" RFC is still on track [to include 422 as the way to express business logic validation problems](https://tools.ietf.org/html/draft-ietf-httpbis-semantics-07#section-9.5.20), so the desire is rooted in some level of standards and the desire to adhere to those semantics.
Skipping past some details, all the `ProblemDetailsFactory` implementations appear to generate their information based on `ModelStateDictionary` contents... but the `ModelStateDictionary` [drops all the strongly-typed exception information](https://github.com/dotnet/aspnetcore/issues/6145) so there's no way to intelligently look for parsing or type conversion errors to differentiate from other validation error types.
Simply intercepting at the [`InvalidModelStateFactory` level](https://github.com/dotnet/aspnetcore/blob/3d042ac9aaf6df4cfd1d2bd4a984a86a45629285/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs#L25) doesn't work because, despite getting the whole `ActionContext` passed in, it's the `ActionContext.ModelState` you end up building from, and that's already set, the exception information lost.
I think **if the exception information could _always be kept_ rather than conditionally thrown out** in favor of only keeping the exception message then we would have something to work with. We could use the exception type to determine if there were any parsing/syntactic issues and go 400 with those; and 422 for everything else.
Contributor guide
Assessment
This issue has not been assessed yet.