OpenAPI ArgumentNullException for type parameter when using parameterized controller route

mkstephenson commented 1 week ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

When attempting to generate an OpenAPI document for a controller with a route that has a version in it managed by the Asp.Versioning library, an ArgumentNullException is thrown when attempting to generate the OpenAPI document.

Expected Behavior

No response

Steps To Reproduce

The code can be found at https://github.com/mkstephenson/net9-openapi-versioning-issue. This is the template WeatherForecast application but with a versioned API. The following changes were made:

Add the Asp.Versioning.Http library (for ASP.NET Core controller projects).
Change the controller route to use the version parameter as documented in https://github.com/dotnet/aspnet-api-versioning/wiki/Versioning-via-the-URL-Path (so the route looks like api/v{version:apiVersion}/[controller]) and add the [ApiVersion(1)] attribute to the same controller
Add builder.Services.AddApiVersioning(); to the Program.cs
Start the project and ensure that the call to the controller still works (the route should be api/v1/weatherforecast)
Call the OpenAPI endpoint (openapi/v1.json) and see the exception page.

Exceptions (if any)

ArgumentNullException: Value cannot be null. (Parameter 'type') System.Text.Json.ThrowHelper.ThrowArgumentNullException(string parameterName) System.Text.Json.Schema.JsonSchemaExporter.GetJsonSchemaAsNode(JsonSerializerOptions options, Type type, JsonSchemaExporterOptions exporterOptions) Microsoft.AspNetCore.OpenApi.OpenApiSchemaService.CreateSchema(OpenApiSchemaKey key) Microsoft.AspNetCore.OpenApi.OpenApiSchemaStore.GetOrAdd(OpenApiSchemaKey key, Func<OpenApiSchemaKey, JsonNode> valueFactory) Microsoft.AspNetCore.OpenApi.OpenApiSchemaService.GetOrCreateSchemaAsync(Type type, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, ApiParameterDescription parameterDescription, bool captureSchemaByRef, CancellationToken cancellationToken) Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetParametersAsync(ApiDescription description, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOperationAsync(ApiDescription description, HashSet capturedTags, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOperationsAsync(IGrouping<string, ApiDescription> descriptions, HashSet capturedTags, IServiceProvider scopedServiceProvider, IOpenApiOperationTransformer[] operationTransformers, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOpenApiPathsAsync(HashSet capturedTags, IServiceProvider scopedServiceProvider, IOpenApiOperationTransformer[] operationTransformers, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) Microsoft.AspNetCore.OpenApi.OpenApiDocumentService.GetOpenApiDocumentAsync(IServiceProvider scopedServiceProvider, CancellationToken cancellationToken) Microsoft.AspNetCore.Builder.OpenApiEndpointRouteBuilderExtensions+<>cDisplayClass0_0+<b__0>d.MoveNext() Microsoft.AspNetCore.Http.Generated.F56B68D2B55B5B7B373BA2E4796D897848BC0F04A969B1AF6260183E8B9E0BAF2GeneratedRouteBuilderExtensionsCore+<>c__DisplayClass2_0+<g__RequestHandler|5>d.MoveNext() Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context) Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context) Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

.NET Version

9.0.100-rc.1.24452.12

Anything else?

No response

mkstephenson commented 1 week ago

I just did another quick test, this appears to occur with any route parameters. So a simple tenant placeholder in the route appears to cause this issue as well. I have updated the reproduction project with this controller as well.

captainsafia commented 6 days ago

@mkstephenson Thanks for filing this issue!

First, I've opened a PR to provide a patch for the issue that you're seeing over at https://github.com/dotnet/aspnetcore/pull/58246. I hope to backport this for .NET 9 so it'll be resolved before we ship GA for November.

Now, some context into why this issue is occurring, as you might suspect the versionId parameter that you've specified in your route doesn't have a concrete parameter associated with it so it isn't possible to infer the type that should be used in order to generate the schema. The implementation relies on the Asp.Versioning library in order to populate a complete ApiParameterDescription type into the ApiExplorer metadata for this API version. The code that handles this mapping for the versioning library lives here. I believe the root cause here is that for whatever reason the logic that sets these parameter types in the versioning library is not being invoked (or is setting the parameter type to null incorrectly).

I just did another quick test, this appears to occur with any route parameters. So a simple tenant placeholder in the route appears to cause this issue as well. I have updated the reproduction project with this controller as well.

The guard I added in the PR above should prevent the exception here, but the same point stands that if you have a scenario where the the parameter is defined only in the route then some other component needs to inform ApiExplorer about the actual type associated with that parameter.

Side note: this is related to the open issue (https://github.com/dotnet/aspnetcore/issues/41773) where there was conversation about the fact that Minimal APIs does not add parameters that only appear in the route to the ApiExplorer metadata unless they also appear in the route handler (and we thus have a parameter type to associate them with). I held the opinion that we should keep this constraint in this comment to avoid issues like this.

Anyhow, I hope this provides some context into why you ran into this issue. The PR I applied will prevent the exception from being thrown but we'll still need to figure out why Asp.Versioning isn't populating the parameter type here.

mkstephenson commented 6 days ago

@captainsafia perfect thanks! For the case where we have other components consuming the parameters (middleware in our case) from the routes and they don't appear directly in the consuming methods, what would be the best way of informing the ApiExplorer of the type of the route parameter? Would it be possible to make a note of this in the docs?

captainsafia commented 2 days ago

@mkstephenson For a situation like that, using a transformer to add the parameter to all operations is probably the best option. The one downside of this is that you'll have to use a transformer (M.A.OpenApi), filter (Swashbuckle), or processor (NSwag) depending on what tool you are using. Each framework has a different API for their "transform an OpenAPI document" feature. If you're going all-in on one tool, this shouldn't be a problem. This approach would make sense if you owned an API.

You can also roll out your own IApiDescriptionProvider implementation that adds the parameter to each route in the application. You'll want to make sure that the parameter type is included (to avoid the problem Asp.Versioning is having here). This approach has the benefit of being friendly across whatever OpenAPI library is used and is the approach I would recommend for a library author.

dotnet / aspnetcore