ProblemDetails is not showing the trace id when AddProblemDetails is set before AddControllers

MigueZS commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

A .NET 8.0 application with ProblemDetails enabled not showing the traceId when AddProblemDetails is called before AddControllers. This happens in both, when using the WebApplicationBuilder or using the CreateDefaultWebhost with an startup class. This only happens when the Accept header sent by the client is application/json.

Expected Behavior

When an exception occurs in one controller, the problem details response shows the traceId or the documentation clearly warns you to call AddProblemDetails after the AddControllers call.

Order is important in middleware but not when configuring services.

Steps To Reproduce

Create a new .net 8.0 application with the following Program.cs

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.

builder.Services.AddControllers();
builder.Services.AddProblemDetails();

var app = builder.Build();

// Configure the HTTP request pipeline.

app.UseExceptionHandler();
app.UseStatusCodePages();

app.UseHttpsRedirection();

app.MapControllers();

app.Run();

Create a controller that throws an exception

[ApiController]
[Route("[controller]")]
public class PDController : ControllerBase
{
    [HttpGet]
    [Route("exception")]
    public IActionResult GetSystemException()
    {
        throw new Exception("This is a test exception");
    }
}

Call the endpoint, the traceId is shown regardless of the Accept header sent

Swap the order of the AddControllers and AddProblemDetails calls in the Program.cs

builder.Services.AddProblemDetails();
builder.Services.AddControllers();

Call the endpoint and the traceId is not shown when the Accept header is "application/json"

Any other Accept header will produce the traceId

Exceptions (if any)

No response

.NET Version

8.0.400

Anything else?

Tested in Visual Studio 2022 or running dotnet run from the console.

Dotnet info output

.NET SDK:
 Version:           8.0.400
 Commit:            36fe6dda56
 Workload version:  8.0.400-manifests.6c274a57
 MSBuild version:   17.11.3+0c8610977

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.400\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
 [aspire]
   Installation Source: VS 17.11.35303.130
   Manifest Version:    8.1.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.1.0\WorkloadManifest.json
   Install Type:        FileBased

Host:
  Version:      8.0.8
  Architecture: x64
  Commit:       08338fcaa5

captainsafia commented 1 month ago

@MigueZS Thanks for reporting this issue!

I believe the problem is a result of the logic that the ProblemDetailsService employs to determine which ProblemDetailsWriter it should use to emit the ProblemDetails object:

https://github.com/dotnet/aspnetcore/blob/d05f3586adc0439595436ff6b8677548302fd64d/src/Http/Http.Extensions/src/ProblemDetailsService.cs#L32-L43

When AddControllers is called, it registers the default implementation used by MVC, which is defined here.

https://github.com/dotnet/aspnetcore/blob/d05f3586adc0439595436ff6b8677548302fd64d/src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs#L277

When AddProblemDetails is called, it registers its own default implementation.

https://github.com/dotnet/aspnetcore/blob/d05f3586adc0439595436ff6b8677548302fd64d/src/Http/Http.Extensions/src/ProblemDetailsServiceCollectionExtensions.cs#L42

The Write logic in each of these is slightly different which is what I suspect is causing this issue. I suspect in this particular case it's MVC's ProblemDetailsWriter that is being called since it doesn't support setting a trace ID. To resolve this, we'll need to update the DefaultProblemDetailsFactory to emit a trace ID as extension data.

Would you be interested in submitting a PR with this fix?

MigueZS commented 1 month ago

Thank you for your response.

I found that your suggestion has already been applied: https://github.com/dotnet/aspnetcore/commit/9f4eb97c12a227edbba99751af3b5ac741634c93. Please correct me if I am wrong.

If so can you please answer the following questions:

The fix was tagged for .net 9.0. Should I suppose the fix will only be available there and never ported back to 8.0?
It is still a little weird that this 2 register 2 different implementations. In that case what it would be the preferred recommended one to use if inverting the calls derives in those 2 different writers?

dotnet / aspnetcore