dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.47k stars 10.03k forks source link

`[ApiController]` attributes causes exception during serialization of `(Validation)ProblemDetails` when using source-generator-based serialization #57019

Closed ssrmm closed 3 months ago

ssrmm commented 3 months ago

Is there an existing issue for this?

Describe the bug

We have a ASP.NET project that both uses source-generator-based serialization and has its API controllers annotated with the [ApiController] attribute. In this combination the HTTP 400 errors generated by [ApiController] for missing or wrong arguments will cause an exception (see first stack trace below) during serialization of the ValidationProblemDetails.

Additionally returning error results such as return BadRequest() or return Unauthorized() also triggers a similar exception (see 2nd stack trace below), this time for the base type ProblemDetails.

Note that removing [ApiController] attribute from the controller makes these exceptions go away, but that is not a desirable solution.

Also note that I've already added builder.Services.AddProblemDetails() to Program.cs, which doesn't seem to have any effect on these cases.

As of .NET 8, it seems you need to add serialization information yourself to prevent the exception:

builder.Services.Configure<JsonOptions>(options => options.SerializerOptions.TypeInfoResolverChain.Add(ProblemDetailsSerializerContext.Default));

[JsonSerializable(typeof(ProblemDetails))]
[JsonSerializable(typeof(HttpValidationProblemDetails))] // Not sure if there is a case that can trigger this type
[JsonSerializable(typeof(ValidationProblemDetails))]
internal sealed partial class ProblemDetailsSerializerContext : JsonSerializerContext;

Expected Behavior

Serialization of (Validation)ProblemDetails should not throw exceptions but instead produce the expected JSON output.

Since (Validation)ProblemDetails is instantiated only internally by ASP.NET, I would expect that the necessary type info for serialization is provided by ASP.NET itself. Having the API consumer provide it, requires many people to duplicate the same code, Also it seems like providing the type on consumer side makes everyone somewhat reliant on implementation details. The exact type derived from ProblemDetails in use is something that presumably may change in the future.

Steps To Reproduce

Run the minimal reproduction and open

Exceptions (if any)

First exception (ValidationProblemDetails):

System.NotSupportedException: JsonTypeInfo metadata for type 'Microsoft.AspNetCore.Mvc.ValidationProblemDetails' was not provided by TypeInfoResolver of type '[]'. If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_NoMetadataForType(Type type, IJsonTypeInfoResolver resolver)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Nullable`1 ensureNotNull, Boolean resolveIfMutable, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfo(Type type)
   at Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|7_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Second exception (ProblemDetails):

System.NotSupportedException: JsonTypeInfo metadata for type 'Microsoft.AspNetCore.Mvc.ProblemDetails' was not provided by TypeInfoResolver of type '[]'. If using source generation, ensure that all root types passed to the serializer have been annotated with 'JsonSerializableAttribute', along with any types that might be serialized polymorphically.
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_NoMetadataForType(Type type, IJsonTypeInfoResolver resolver)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Nullable`1 ensureNotNull, Boolean resolveIfMutable, Boolean fallBackToNearestAncestorType)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfo(Type type)
   at Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|7_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

.NET Version

8.0.303

Anything else?

There seem to be several related issues, but they are all slightly different, dealing either with different types or affecting a different scenario:

captainsafia commented 3 months ago

@ssrmm Thanks for filing the issue. I believe the issue you're running into might be related to the way that your JsonSerializaerOptions are configured.

The Configure<JsonOptions> that you are using modifies the JSON options object that is used by minimal APIs. For controllers, you'll want to use something like the sample below that uses the AddJsonOptions API for controller-based APIs.

Can you try it and let me know if it works for you?

Also, we can probably update the docs to be more clear about what each options object influences. Would you be interested in tweaking the doc strings on this type and and this one to clarify what frameworks they end up being used for?

using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddControllers()
    .AddJsonOptions(options => options.JsonSerializerOptions.TypeInfoResolverChain.Add(ProblemDetailsSerializerContext.Default));
builder.Services.AddProblemDetails();

var app = builder.Build();
app.MapControllers();
app.UseDeveloperExceptionPage();
app.Run();

[JsonSerializable(typeof(ProblemDetails))]
[JsonSerializable(typeof(HttpValidationProblemDetails))] // Not sure if there is a case that can trigger this type
[JsonSerializable(typeof(ValidationProblemDetails))]
internal sealed partial class ProblemDetailsSerializerContext : JsonSerializerContext;
ssrmm commented 3 months ago

Well I guess that works in the sense that AddJsonOptions() can be used to configure the options. But that is also true for my original Configure<JsonOptions>(). The fact that I was using the function call from the "wrong" API is good to know, but it does not solve what I perceive to be the actual issue.

As per my explanation in the "Expected Behavior" I would expect not having to call either function in this case. The fact that [ApiController] will use ProblemDetails and derived classed somewhere under the hood is a implementation detail that consumers of the ASP.NET APIs should not have to know about.

The Configure<JsonOptions>() in the issue description was just the workaround that I found for that. Note that the reproduction code that I linked to in my original post does not contain Configure<JsonOptions>() at all.

captainsafia commented 3 months ago

@ssrmm Yep, we do automatically configure the JsonSerializerContext for ProblemDetails that is defined here when you configure the ProblemDetails service using AddProblemDetails defined (here).

The ProblemDetails-service APIs were primarily designed with minimal APIs in mind. They do work with controller-based APIs but there's a few usability issues that would make me cautious about recommending it to you as an option.

ssrmm commented 3 months ago

@captainsafia This still doesn't solve the issue though.

As I said earlier AddProblemDetails() does not have any effect in this scenario. I can add it or leave it out, [ApiController] will generate exceptions somewhere in framework code either way. I simply mentioned it in my original post because I assumed otherwise the first question would have been if I had tried using AddProblemDetails().

The only things I could find that helps in this situation is to define my own JsonSerializerContext for those types, which I don't think is reasonable for the reasons I explained in the earlier posts of this thread.

captainsafia commented 3 months ago

As I said earlier AddProblemDetails() does not have any effect in this scenario. I can add it or leave it out, [ApiController] will generate exceptions somewhere in framework code either way. I simply mentioned it in my original post because I assumed otherwise the first question would have been if I had tried using AddProblemDetails().

Yes, to be clear, what I intended to communicate here was that AddProblemDetails configures the JsonOptions instance that minimal APIs uses. So, as long as you are using minimal APIs, the JSON serializer context is configured correctly.

It doesn't configure the MVC JsonOptions object because that is defined in the Mvc assembly and we try to avoid taking dependencies on MVC in more lower level dependencies (like Microsoft.AspNetCore.Http.Extensions in this case).

FWIW, I totally get that this is bothersome if you are trying to configure problem details and source generated-STJ in an MVC application but we don't have good out-of-the-box options for resolving this given the dependency hierarchy problem I mentiond earlier.

ssrmm commented 3 months ago

Yes, to be clear, what I intended to communicate here was that AddProblemDetails configures the JsonOptions instance that minimal APIs uses. So, as long as you are using minimal APIs, the JSON serializer context is configured correctly.

I guess then I misread this:

The ProblemDetails-service APIs [...] work with controller-based APIs but [caveats].

I get that you can't just reference higher-level assemblies from lower-level assemblies. I didn't mean to suggest that AddProblemDetails() should handle this. I didn't even intend to suggest any solution at all, because I don't have any idea what a solution would even look like.

FWIW, I totally get that this is bothersome if you are trying to configure problem details and source generated-STJ in an MVC application but we don't have good out-of-the-box options for resolving this

Shouldn't this issue then be about improving support for this scenario? Or is the statement that there is no good support and there are no plans to improve it?