search

dotnet-architecture / eShopOnContainers

Cross-platform .NET sample microservices and container based application that runs on Linux Windows and macOS. Powered by .NET 7, Docker Containers and Azure Kubernetes Services. Supports Visual Studio, VS for Mac and CLI based environments with Docker CLI, dotnet CLI, VS Code or any other code editor. Moved to https://github.com/dotnet/eShop.
https://dot.net/architecture
24.53k stars 10.37k forks source link

Missing swagger operationfilter for GlobalException? #602

Closed Eneuman closed 6 years ago

Eneuman commented 6 years ago

Some of the services uses a GlobalExceptionFilter that returns a internal server error object. There is currently no Swagger Operation decorator that adds Http Status Code 500 as a valid response. This will make client generators (like auto rest and nswag) create code that does not handle the response correctly.

I can make a PR if you decide that this should be fixed.

CESARDELATORRE commented 6 years ago

We're in the middle of important updates heading to .NET Core 2.1 and ASP.NET Core 2.1. We'll have it merged during May, please, let's make this exceptions improvements PR afterwards, around early June, ok? Thanks a lot for your help! 👍

Eneuman commented 6 years ago

Sounds good, i'll put this on hold until then :)

Eneuman commented 6 years ago

Since, you are moving to .Net Core 2.1, maby it would be a better idea to create a new "ServerProblemDetails" response that conforms with RFC 7807. That would go well with the new "ValidationProblemDetails" introduced in .Core 2.1 The GlobalExceptionFilter can then return the "ServerProblemDetails" response and a "ServerProblemOperationFilter" can hook it up to Swagger, by adding the correct response type (500) for all Operations.

mvelosop commented 6 years ago

Hi @Eneuman, just wondering if you're still looking forward to get into this endeavour, now that the .NET Core 2.1 version of eShopOnContainers has been released.

I'm about to get this issue into the possible future feature section in the Roadmap and close it, but just wanted to check first if you are still planning (and have time) to get into this.

Thanks.

mvelosop commented 6 years ago

Added issue to possible feature section in the Roadmap