[dotnet-sdk-9.0.100-rc.1.24409.1] No "400 - bad" ErrorResponse shows in CleanArchitecture app

Junjun-zhao commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

When run the 3rd party application with the latest .NET 9 build, there is no 400 - bad request in /swagger/index.html. it passed with dotnet-sdk-9.0.100-preview.7.24402.8 and fails with dotnet-sdk-9.0.100-rc.1.24409.1

Application Name: CleanArchitecture1 OS： Windows 10 21H2 CPU: X64 .NET Build Number: dotnet-sdk-9.0.100-rc.1.24409.1 App & Source checking at: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2208398 Github Link:https://github.com/ardalis/CleanArchitecture

Verify Scenarios: 1). Windows 10 21H2 AMD64 + dotnet-sdk-8.0.400: Pass 2). Windows 10 21H2 AMD64 + dotnet-sdk-9.0.100-preview.7.24402.8: Pass 3). Windows 10 21H2 AMD64 + dotnet-sdk-9.0.100-rc.1.24409.1: Fail

Expected Behavior

400-Bad Request show in Responses and there is ErrorResponse in schemas.

Steps To Reproduce

App Repro Steps

Copy the app to your local machine and change the CleanArchitecture1\Clean.Architecture.Web.runtimeconfig.json to let the app run against with dotnet-sdk-9.0.100-rc.1.24409.1.

"frameworks": [
  {
    "name": "Microsoft.NETCore.App",
    "version": "9.0.0-rc.1.24408.12"
  },
  {
    "name": "Microsoft.AspNetCore.App",
    "version": "9.0.0-rc.1.24407.10"
  }
]

Launch CleanArchitecture1\Clean.Architecture.Web.exe.
Open edge to http://localhost:5000/swagger/index.html.

Actual Result: 400-Bad Request was disappeared and there is no ErrorRseponse in schemas.

Minimal Repro Steps (Demo attached):

Create a default8.0 asp web api.
Install FastEndpoints.Swagger 5.28.0.5-beta.

Copy the following code to replace code in Program.cs.


using FastEndpoints;
using FastEndpoints.Swagger;
var builder = WebApplication.CreateBuilder(args);
builder.Services.AddFastEndpoints().SwaggerDocument();
var app = builder.Build();
app.UseFastEndpoints().UseSwaggerGen();
app.Run();

Create MyEndpoint.cs with below code. using FastEndpoints; namespace ApiDemo { public class MyEndpoint : Endpoint<MyRequest, MyResponse> { public override void Configure() { Post("/Test"); Description(b => b.ProducesProblemDetails(400)); } public override async Task HandleAsync(MyRequest r, CancellationToken c) { await SendAsync(new() { Name = "Test" }); } } public class MyRequest { public int Id { get; set; } } public class MyResponse { public string Name { get; set; } } }

Change the runtime.config file to let the app run against with dotnet-sdk-9.0.100-rc.1.24409.1.

"frameworks": [
  {
    "name": "Microsoft.NETCore.App",
    "version": "9.0.0-rc.1.24408.12"
  },
  {
    "name": "Microsoft.AspNetCore.App",
    "version": "9.0.0-rc.1.24407.10"
  }
]

Launch the project.
Go to "http://localhost:5000/swagger/index.html". (If launch by vs, it will redirect to the page automatically.)

Expected Result: 400-Bad Request show in Responses and there are 4 items in schemas.

Actual Result: 400-Bad Request was disappeared and there are 2 items in schemas.

Exceptions (if any)

No response

.NET Version

9.0.100-rc.1.24409.1

Anything else?

Dotnet Info: .NET SDK: Version: 9.0.100-rc.1.24409.1 Commit: 43360291a5 Workload version: 9.0.100-manifests.f198e26c MSBuild version: 17.12.0-preview-24407-03+6bc91d5e2

Runtime Environment: OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\9.0.100-rc.1.24409.1\

Host: Version: 9.0.0-rc.1.24408.12 Architecture: x64 Commit: static

.NET SDKs installed: 9.0.100-rc.1.24409.1 [C:\Program Files\dotnet\sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 9.0.0-rc.1.24407.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 9.0.0-rc.1.24408.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 9.0.0-rc.1.24408.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@dotnet-actwx-bot @dotnet/compat

Junjun-zhao commented 4 weeks ago

@javiercn Could you please take a look at and help triage this issue? Please help move the right area if this is not correct. Thanks very much.

adityamandaleeka commented 4 weeks ago

@captainsafia

captainsafia commented 4 weeks ago

Took a look at this.

It looks like FastEndpoint's Swagger/OpenAPI implementation takes a direct dependency on NSwag for generating Open API documents. So the new Microsoft.AspNetCore.OpenApi implementation isn't coming into play here.

FastEndpoints does take a dependency on the EndpointMetadataApiDescriptionProvider implementation in ApiExplorer that is used to create intermediary metadata for minimal APIs. My immediate suspicion is that the changes that we made in this class in 9.0-preview.7 to support native AoT might be causing an impact here.

Targeting the 9.0-preview.6 runtime confirms this suspicion since this bug doesn't repro there.

I'll have to take a closer look at why FastEndpoint's ProducesProblemDetails implementation produces metadata that doesn't play nice with ApiExplorer given test coverage for the ProducesProblem extension method in minimal APIs.

captainsafia commented 3 weeks ago

I'll have to take a closer look at why FastEndpoint's ProducesProblemDetails implementation produces metadata that doesn't play nice with ApiExplorer given test coverage for the ProducesProblem extension method in minimal APIs.

OK, I figured out what is going on. The issue is in this new bit of code:

https://github.com/dotnet/aspnetcore/blob/6b047bc57f9f9d3f015f348caad64f5d1bd3f0de/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs#L335-L341

It happens that the ProducesProblemDetails method that FastEndpoints calls adds ProducesResponseTypeMetadata to a type that implements IResult:

https://github.com/FastEndpoints/FastEndpoints/blob/422567a5a454e5cf103a052276ad2391acfbcf9b/Src/Library/DTOs/ProblemDetails.cs#L22-L23

We could fix this on our end by adding an exception to the check above that respects the response type if it implements IEndpointMetadataProvider.

I'll take a look at fixing this although I don't think this merits taking a fix through tactics for RC1/RC2 since there are workarounds. We can see if this manifests after GA and backport the fix via servicing if so.

Junjun-zhao commented 3 weeks ago

Thanks @captainsafia for looking into this issue. Could you please share the workarounds mentioned?

I'll take a look at fixing this although I don't think this merits taking a fix through tactics for RC1/RC2 since there are workarounds. We can see if this manifests after GA and backport the fix via servicing if so.

captainsafia commented 3 weeks ago

@Junjun-zhao You can try the following using the ProblemDetails definition in MVC:

using FastEndpoints;
using FastEndpoints.Swagger;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Mvc.ApiExplorer;

var builder = WebApplication.CreateBuilder(args);

// builder.Services.AddOpenApi();

builder.Services.AddFastEndpoints().SwaggerDocument(o => o.ShortSchemaNames = true);

var app = builder.Build();

app.UseFastEndpoints().UseSwaggerGen();

app.Run();

namespace ApiDemo
{
    public class MyEndpoint : Endpoint<MyRequest, MyResponse>
    {
        public override void Configure()
        {
            Post("/Test");
            Description(b => b.Produces<Microsoft.AspNetCore.Mvc.ProblemDetails>(400));
        }
        public override async Task HandleAsync(MyRequest r, CancellationToken c)
        {
            await SendAsync(new()
            {
                Name = "Test"
            });
        }
    }
    public class MyRequest
    {
        public int Id { get; set; }
    }
    public class MyResponse
    {
        public string Name { get; set; }
    }
}

Also, I'm inclined to says that we should be perspective about supporting IResult implementations that don't directly serialize themselves in the output but serialize a provided input, similar to the pattern used for ProblemHttpResult.

dotnet / aspnetcore