Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of ProblemDetails is inconsistent throughout ASP.NET Core #32957

Closed
dozer75 opened this issue May 24, 2021 · 5 comments · Fixed by #42384
Closed

Usage of ProblemDetails is inconsistent throughout ASP.NET Core #32957

dozer75 opened this issue May 24, 2021 · 5 comments · Fixed by #42384
Assignees
Labels
area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels cost: L Will take from 5 - 10 days to complete enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:2 Work that is important, but not critical for the release triage-focus Add this label to flag the issue for focus at triage
Milestone

Comments

@dozer75
Copy link

dozer75 commented May 24, 2021

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
Copy link
Author

dozer75 commented May 24, 2021

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

@javiercn javiercn added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-API-Explorer labels May 24, 2021
@mkArtakMSFT mkArtakMSFT added the bug This issue describes a behavior which is not expected - a bug. label May 24, 2021
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone May 24, 2021
@ghost
Copy link

ghost commented May 24, 2021

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.

@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed bug This issue describes a behavior which is not expected - a bug. labels May 24, 2021
@pranavkm pranavkm added area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Oct 19, 2021
@ghost
Copy link

ghost commented Dec 28, 2021

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.

@rafikiassumani-msft rafikiassumani-msft added triage-focus Add this label to flag the issue for focus at triage Priority:2 Work that is important, but not critical for the release cost: L Will take from 5 - 10 days to complete labels Jan 11, 2022
@commonsensesoftware
Copy link

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
Copy link
Member

brunolins16 commented Jun 1, 2022

Related issue #4953

@brunolins16 brunolins16 linked a pull request Jul 19, 2022 that will close this issue
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels cost: L Will take from 5 - 10 days to complete enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:2 Work that is important, but not critical for the release triage-focus Add this label to flag the issue for focus at triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants