Skip to content

DefaultApiConventions are missing Status400BadRequest responses for GET requests #9375

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

Open
cremor opened this issue Apr 15, 2019 · 3 comments
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-openapi severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@cremor
Copy link
Contributor

cremor commented Apr 15, 2019

Describe the bug

The DefaultApiConventions should define a Status400BadRequest response for Get and Find methods.

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of ASP.NET Core '2.2'
  2. Create a Web API project
  3. Apply the [ApiConventionType(typeof(DefaultApiConventions))] attribute to the controller or assembly.
  4. Analyse the structure returned by ApiExplorer interfaces (or add Swashbuckle.AspNetCore to quickly see the same data in Swagger-UI).
  5. Check the possible responses of api/values/{id} methods. Note that Status400BadRequest is not defined as a possible response.
  6. Call the api/values/{id} with a string as id. Note the response ("400 BadRequest").

Expected behavior

Becaues GET can return "400 BadRequest", there should be a Status400BadRequest response defined for GET methods, just like there is for all POST, PUT and DELETE.

Additional context

I know that these conventions were created after the default scaffolding template "Web API Controller with EF Core context", which doesn't return BadRequest in Get. But neither does Delete, which accepts the same parameter as Get. And thanks to automatic HTTP 400 responses both Get and Delete will return "400 BadRequest" if model binding fails (e.g. if you call the action with a string parameter but your id is an integer).

Output of dotnet --info:

Click to expand .NET Core SDK (gemäß "global.json"): Version: 2.2.106 Commit: aa79b139a8

Laufzeitumgebung:
OS Name: Windows
OS Version: 10.0.16299
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\2.2.106\

Host (useful for support):
Version: 2.2.4
Commit: f95848e524

.NET Core SDKs installed:
2.1.505 [C:\Program Files\dotnet\sdk]
2.2.106 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
https://aka.ms/dotnet-download

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 16, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @cremor.
@glennc what needs to be done here?

@cremor
Copy link
Contributor Author

cremor commented Dec 6, 2019

Since #8855 is now both closed and locked, please don't forget it when working on this issue. As per comment from @pranavkm:

The most likely solution we would go with would be to create a separate convention and have users explicitly opt in. If you'd like we could use #9375 to consider fixing both of these using this approach.

@mkArtakMSFT
Copy link
Member

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

@javiercn javiercn added affected-very-few This issue impacts very few customers severity-nice-to-have This label is used by an internal tool enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Feb 19, 2021 — with ASP.NET Core Issue Ranking
@pranavkm pranavkm added old-area-web-frameworks-do-not-use *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
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-openapi severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants