dotnet/aspnetcore

Usage of ProblemDetails is inconsistent throughout ASP.NET Core

dozer75 opened this issue · 5 comments

dozer75 commented

Describe the bug

The usage of the ProblemDetails is inconsistent throughout the library. There is no green line when it is actually used and not when using standard ASP.NET Core logic. In addition the flag SuppressMapClientErrors is equally inconsistent in its usage as well.

The documentation states:

When the compatibility version is 2.2 or later, MVC transforms an error result (a result with status code 400 or higher) to a result with ProblemDetails.

It also states that:

The automatic creation of a ProblemDetails for error status codes is disabled when the SuppressMapClientErrors property is set to true

In my experiments (I must admit I haven't tried ALL scenarios) I found:

  • NotFound and Unauthorized applies ProblemDetails dynamically based on SuppressMapClientErrors setting.
  • ValidationProblem and Problem will always apply ProblemDetails no matter of the SuppressMapClientErrors setting.
  • BadRequest and UnprocessableEntity applies ProblemDetails dynamically based on SuppressMapClientErrors setting
    • However; when passing ModelState to these they always plain ModelState result.
  • Unhandled exceptions in controllers does not apply ProblemDetails information both in Development and Production mode.

Expected results

Basically:

Applying ProblemDetails to all results above 400 should be based on the SuppressMapClientErrors setting.

  • BadRequest and UnprocessableEntity should always handle result the same way no matter input. That is, honor the SuppressMapClientErrors setting and return ProblemDetailsbased on that.
  • Unhandled exceptions should honor the SuppressMapClientErrors as well.
  • The Problem should in my opinion apply ProblemDetails based on SuppressMapClientErrors since this is the easiest call to return status codes that other methods doesn't cover.

I also find it a bit strange that there is a ValidationProblem as both BadRequest and UnprocessableEntity covers this (if they had been working). Is it REALLY necessary if BadRequest and UnprocessableEntity had consistent handling?

To Reproduce

  • Clone this repository
  • Run it
  • Answer the questions about enabling/disabling client error mapping and developer exception handling during startup
  • Try out the different controller actions available in the swagger and see the problem description

Further technical details

ASP.NET Core version 5.0

dotnet --info

NET SDK (reflecting any global.json):
Version: 5.0.200
Commit: 70b3e65d53

Runtime Environment:
OS Name: Windows
OS Version: 10.0.19041
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\5.0.200\

Host (useful for support):
Version: 5.0.3
Commit: c636bbdc8a

.NET SDKs installed:
5.0.100 [C:\Program Files\dotnet\sdk]
5.0.200 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.All 2.1.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Visual Studio

Microsoft Visual Studio Enterprise 2019
Version 16.9.0
VisualStudio.16.Release/16.9.0+31025.194
Microsoft .NET Framework
Version 4.8.04084

dozer75 commented

Just discovered #4953, which this issue is related to, but this issue has some other information as well around this.

msftbot commented

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

msftbot commented

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

commonsensesoftware commented

To add to this list, which has been discovered via API Versioning, the following do not always provide or enable any hooks for ProblemDetails in the routing system today:

  • 404
  • 405
  • 415

This happens any time a built-in MatcherPolicy creates its own Endpoints and destinations land there. Developers and library authors should have a chance to participate.

It's a bit of an aside, but I suspect part of this behavior is due to the fact that ProblemDetails is part of the core HTTP stack, but ProblemDetailsFactory is abstract class in MVC core. Adding a new mechanism in the core HTTP layer (an interface?) that can be optionally resolved from DI to provide ProblemDetails would help complete the picture. There maybe other options, but an interface would provide an option service at the core HTTP layer for things like Minimal APIs and allow MVC core or MVC apps to continue going through ProblemDetailsFactory which would implement said new interface.

I had to provide my own interface for API Versioning in .NET 6 to bridge this gap. I'm sure others would benefit from being able to generate ProblemDetails from the core HTTP stack up.

brunolins16 commented

Related issue #4953