[Bug]: Exception in SwaggerGen for IQueryable<T> result (OData) upgrading v6.7.2 to v6.7.3
Closed this issue · 10 comments
Describe the bug
Updating from v6.7.2 to v6.7.3, controller actions with IQueryable crash SwaggerGen with "Index out of bound" (see exception below).
T is a OData model which, during setup, has been configured to "Ignore" a given Property during ModelBuilding with b.Ignore(p => p.Prop)
The OData model is integrated with the AspNetCore ApiDescription and Asp.Versioning.
Expected behavior
As v6.7.2
Actual behavior
Exception when requesting /swagger/docs/v1.0 (json schema generation) leading to 500.
Steps to reproduce
Install and configure AspNetCore OData with Asp.Versioning.
Configure in a IModelConfiguration for OData one property to be ignored: b.Ignore(p => p.Prop)
A non-minimal reproduction is available here: https://github.com/ARKlab/Ark.Tools/tree/af4e51d959a9a11d24d101f76f977a59422b8dfa/Samples/WebApplicationDemo
Commenting this line disable the issue: https://github.com/ARKlab/Ark.Tools/blob/af4e51d959a9a11d24d101f76f977a59422b8dfa/Samples/WebApplicationDemo/Configuration/PersonModelConfiguration.cs#L19
Exception(s) (if any)
Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Failed to generate Operation for action - WebApplicationDemo.Controllers.V1.PeopleController.Get (WebApplicationDemo). See inner exception
---> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Failed to generate schema for type - System.Linq.IQueryable`1[WebApplicationDemo.Dto.Person]. See inner exception
---> System.IndexOutOfRangeException: Index was outside the bounds of the array.
at NullabilityInfo System.Reflection.NullabilityInfoContext.Create(PropertyInfo propertyInfo)
at NullabilityInfo Swashbuckle.AspNetCore.SwaggerGen.MemberInfoExtensions.GetNullabilityInfo(MemberInfo memberInfo)
at bool Swashbuckle.AspNetCore.SwaggerGen.MemberInfoExtensions.IsNonNullableReferenceType(MemberInfo memberInfo)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateSchemaForMember(Type modelType, SchemaRepository schemaRepository, MemberInfo memberInfo, DataProperty dataProperty)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.CreateObjectSchema(DataContract dataContract, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository)+() => { } [3]
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateReferencedSchema(DataContract dataContract, SchemaRepository schemaRepository, Func<OpenApiSchema> definitionFactory)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateSchemaForType(Type modelType, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateSchema(Type modelType, SchemaRepository schemaRepository, MemberInfo memberInfo, ParameterInfo parameterInfo, ApiParameterRouteInfo routeInfo)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.CreateArraySchema(DataContract dataContract, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository)+() => { } [1]
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateSchemaForType(Type modelType, SchemaRepository schemaRepository)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SchemaGenerator.GenerateSchema(Type modelType, SchemaRepository schemaRepository, MemberInfo memberInfo, ParameterInfo parameterInfo, ApiParameterRouteInfo routeInfo)
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateSchema(Type type, SchemaRepository schemaRepository, PropertyInfo propertyInfo, ParameterInfo parameterInfo, ApiParameterRouteInfo routeInfo)
--- End of inner exception stack trace ---
at OpenApiSchema Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateSchema(Type type, SchemaRepository schemaRepository, PropertyInfo propertyInfo, ParameterInfo parameterInfo, ApiParameterRouteInfo routeInfo)
at OpenApiMediaType Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.CreateResponseMediaType(Type modelType, SchemaRepository schemaRespository)
at OpenApiResponse Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateResponse(ApiDescription apiDescription, SchemaRepository schemaRepository, string statusCode, ApiResponseType apiResponseType)+(string contentType) => { } [2]
at Dictionary<TKey, TElement> System.Linq.Enumerable.ToDictionary<TSource, TKey, TElement>(IEnumerable<TSource> source, Func<TSource, TKey> keySelector, Func<TSource, TElement> elementSelector, IEqualityComparer<TKey> comparer) x 2
at OpenApiResponse Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateResponse(ApiDescription apiDescription, SchemaRepository schemaRepository, string statusCode, ApiResponseType apiResponseType)
at OpenApiResponses Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateResponses(ApiDescription apiDescription, SchemaRepository schemaRepository)
at async Task<OpenApiOperation> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateOperationAsync(ApiDescription apiDescription, SchemaRepository schemaRepository, Func<ApiDescription, SchemaRepository, Task<List<OpenApiParameter>>> parametersGenerator, Func<ApiDescription, SchemaRepository, Task<OpenApiRequestBody>> bodyGenerator, Func<OpenApiOperation, OperationFilterContext, Task> applyFilters)
--- End of inner exception stack trace ---
at async Task<OpenApiOperation> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateOperationAsync(ApiDescription apiDescription, SchemaRepository schemaRepository, Func<ApiDescription, SchemaRepository, Task<List<OpenApiParameter>>> parametersGenerator, Func<ApiDescription, SchemaRepository, Task<OpenApiRequestBody>> bodyGenerator, Func<OpenApiOperation, OperationFilterContext, Task> applyFilters) x 2
at async Task<Dictionary<OperationType, OpenApiOperation>> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateOperationsAsync(IEnumerable<ApiDescription> apiDescriptions, SchemaRepository schemaRepository)
at async Task<OpenApiPaths> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GeneratePathsAsync(IEnumerable<ApiDescription> apiDescriptions, SchemaRepository schemaRepository, Func<IGrouping<string, ApiDescription>, SchemaRepository, Task<Dictionary<OperationType, OpenApiOperation>>> operationsGenerator) x 2
at async Task<OpenApiDocument> Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwaggerAsync(string documentName, string host, string basePath)
at async Task Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
at async Task Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
at async Task Hellang.Middleware.ProblemDetails.ProblemDetailsMiddleware.Invoke(HttpContext context)
Swashbuckle.AspNetCore version
6.7.3
.NET Version
net8.0
Anything else?
No response
/cc @patrikwlund Any ideas on this one? The exception seems to be coming from .NET itself.
My hunch is that the exception is coming from here.
If that's the case, it seems to suggest that the property in question somehow has a setter property with no parameters (parameters[0 - 1] // boom).
If this is the case, then this is an issue in .NET itself.
Yep, the bug is coming from here for System.String set_Email() on WebApplicationDemo.Dto.Person.
I don't know why the setter seems to have no parameters in this scenario, but it doing so hits a bug in .NET: dotnet/runtime#107033
Unless there's something weird we're somehow doing that causes that when it shouldn't be the case, then this isn't anything we can fix in Swashbuckle.
I tried to create a minimal repro, but this works as expected:
using System.ComponentModel.DataAnnotations;
using System.Reflection;
var type = typeof(Person);
var property = type.GetProperty(nameof(Person.Email));
var context = new NullabilityInfoContext();
var info = context.Create(property!);
Console.WriteLine(info!.ToString());
public class Person
{
public int Id { get; set; }
[Required]
[StringLength(25)]
public string? FirstName { get; set; }
[Required]
[StringLength(25)]
public string? LastName { get; set; }
public string? Email { get; set; }
public string? Phone { get; set; }
}The above code doesn't throw, but prints System.Reflection.NullabilityInfo to the console.
@martincostello thanks for the investigation. 👍
How ends up in the set_Email() ?
If I comment the person.Ignore(p => p.Phone); link it works.
The bug is somewhat tied to the ApiDescription's PropertyInfo which is probably mangled by the ModelBuilder in respect from a straight class reflection.
The endpoint /swagger/docs/v2.0 works, as in that case the Phone is not ignored (see the if (version < 2)). The Person class is the same and has both the Email and Phone properties.
It looks to me that somehow the PropertyInfo passed is mangled (maybe by ApiDescription+OData ModelBuilder?) which triggers the bug in the runtime.
Is there any way to disable the change made from v6.7.2 to v6.7.3 that triggered this (aside not upgrading)?
How ends up in the set_Email() ?
Sorry, I don't know. Hopefully the folks in the dotnet/runtime will shed some light on it. I don't know how it could be mangled as it's obtained from the runtime through reflection.
Is there any way to disable the change made from v6.7.2 to v6.7.3 that triggered this (aside not upgrading)?
Unfortunately not. Maybe when we know more about why this is happening there might be away to work around the issue.
Wow! Now that was unexpected. I have never used these libraries before, but they seem to generate new types during runtime using that configuration.
When I catch the exception and inspect the property that causes the error, I reach the same conclusion as @martincostello. Looking closer, I indeed see that the email setter method has zero parameters, which doesn't exactly seem "legal".
If we instead run person.Ignore(p => p.Email);, then it throws because Phone's setter has no parameters. Ignoring none or both works, but not just one. Maybe no runtime type is generated if we don't do any ignores, so we don't get the error then.
My current conclusion
If zero parameters is somehow legal, then it's a bug in NullabilityInfoContext. If not, then the types are likely dynamically generated in an "illegal" way.
Workarounds
Ignoring both Email and Phone seems to work. There may also be other ways to configure these properties in a way that generates them nicely.
Maybe @commonsensesoftware has some insight here if this is something API Versioning is doing with dynamically generated types.
The API Explorer extensions for OData provided by API Versioning does use dynamically generated types in some scenarios. This feature is known as Model Substitution. The long and short of it is that the EDM defines how a model is defined over the wire. Since EDMs are version-specific, so too are their associated models.
Like most OpenAPI generators, Swashbuckle works from Reflection which is based on the type information reported by the ApiDescription. In any other context, this would require the API author to create an unique type specific to each API version. OData is more flexible. Since the EDM defines what the model must look like in a particular API version, the API Explorer extensions can make a single type match what the EDM says. It does this by creating a dynamically generated, but not executable, type which is a subset of the original type according what the EDM defines. Any attributes defined are also copied to the generated type. The substituted model is then applied to the ApiDescription. When Swashbuckle inspects the type, it doesn't know that it is dynamically generated. It doesn't really care either. It only looks at the member name, types, and attributes, which are retained. This allows the author to a have one model class that can look differently in different API versions. If all of the type members and the EDM definition are exactly the same, then no substitution takes place; the original type is used.
A similar process is used to create a type for OData actions because ODataActionParameters is nothing more than Dictionary<string, object>. The dynamically generated type emits what the action parameters type is supposed to look like according to the EDM from the perspective of a consumer of the API Explorer, such as Swashbuckle.
It is not possible to opt out of this feature via configuration, but you can change the behavior by replacing the IModelTypeBuilder service. It's pretty complicated so I doubt you want to go down that road. You could have an implementation that always echoes back the original type, but it probably will not give you want you want as an end result. The easiest way to work around any issue is to use a type that exactly models how it should look to the API Explorer, OpenAPI, and ultimately Swashbuckle. When you use Ignore(t => t.Property) you are ignoring and hiding the property from the EDM and what's emitted over the wire, but it is not hidden from the API Explorer, OpenAPI, or Swashbuckle because they only use Reflection and don't know anything about the EDM.
This feature has been in play for years. Non-OData users have wanted a similar feature for POCOs, but it is a very nontrivial implementation. It is entirely possible the there is some edge case that has now broken. Code generation is tricky business. In some future state, I'd love to use Source Generators to precompile the variants which would be faster and AOT-friendly. Again, that's a pretty complex process.
@AndreaCuneo feel free to open an issue in the API Versioning repo with your repro. I've been really backed up with life and my real job of late, but I'll take a deeper look as soon as I can. Again, the easiest escape hatch is to define your type/class for the EDM. For example, you might have V1.Person and V2.Person, Inheritance shouldn't be a problem if you want to go that route. At a minimum, I hope that provides some context and unblocks you.
Thanks for looking into it.
I'm going to close this issue as it's external.