-
Notifications
You must be signed in to change notification settings - Fork 10.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using the System.Text.Json source generator with ASP NET Core throws NotSupportedException when attempting to serialize IEnumerable<T> #43894
Comments
I think the root cause will be the same as #43236, which is due to changes to S.T.J itself. |
I don't think that this is the same underlying issue. In #43236, the source generated context has no direct reference to the underlying ProblemDetails type, whereas is this case the context has all the information it needs (we have explicitly opted in with a There should be no need for a custom type resolver in this case. Interestingly, using a |
Another data point - a nested IEnumerable property serializes successfully without any errors. [JsonSerializable(typeof(NestedEnumerable))]
public partial class JsonContext : JsonSerializerContext { }
//...
public class NestedEnumerable
{
public IEnumerable<MyClass> NestedIEnumerable { get; set; } = null!;
}
//...
[HttpGet("nested")]
public IActionResult Get2()
{
var l2 = new NestedEnumerable()
{
NestedIEnumerable = new List<MyClass>() { new(), new(), new() }
};
return Ok(l2);
} |
The issue is This explicit call: var output = JsonSerializer.Serialize(l, JsonContext.Default.IEnumerableMyClass); Is not the moral equivalent of what MVC is doing. The generic is inferred as The equivalent would be this: var options = new JsonSerializerOptions(JsonSerializerDefaults.Web);
options.AddContext<JsonContext>();
await System.Text.Json.JsonSerializer.SerializeAsync(new MemoryStream(), l, l.GetType(), options); Which also throws. MVC passes the concrete type (which is List<MyClass>) to serialize. TL;DR change your code to this: [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
[JsonSerializable(typeof(List<MyClass>))]
public partial class JsonContext : JsonSerializerContext
{
} And it should work. |
@davidfowl, in this simplified example I know exactly what the underlying type is, and could use The important thing to realise is that this surfaces as a regression and will break existing code that updates to .NET 7.0. I also think its totally reasonable that we might want to use the abstraction Good point about async serialization - I was thinking about that yesterday, that, of course, ASP .NET Core is going to be using the async serialization code path. I'll open another issue in the .NET runtime repo to surface this change in behaviour, thanks for your help! |
Why are you using the source generator at all? |
Just to get experience using it in a personal project - I don't really need the additional perf in this case. I think this behavioural change might be most likely to surface in performance focused libraries that would want to be a) high performance and b) trim safe, that use async serialization behind the scenes. |
The change is by design BTW dotnet/runtime#71714. You need to enumerate all types that will be used for serialization when using the source generator and it will no longer fall back to reflection based when it can't find the type information. In .NET 6 it did fallback and it was working because of that (and you didn't achieve trim safety as a result). In .NET 7 now it'll let you know when your type closure is incomplete and needs to be spelling out more (like in this example).
ASP.NET Core itself isn't trim safe so I'm not sure how much this matters for apps today. |
One last question. Out of curiosity, what do minimal APIs do differently that means that the above does not repro when using minimal APIs? |
Minimal APIs doesn't use MVC's JSON serializer options. So if you do this: using System.Text.Json.Serialization;
var builder = WebApplication.CreateBuilder(args);
builder.Services.ConfigureHttpJsonOptions(o =>
{
o.SerializerOptions.AddContext<JsonContext>();
});
var app = builder.Build();
app.MapGet("/", () =>
{
var l = new List<MyClass> { new(), new() };
return Results.Ok(l);
});
app.Run();
[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
[JsonSerializable(typeof(IEnumerable<MyClass>))]
public partial class JsonContext : JsonSerializerContext
{
}
public class MyClass
{
public int MyProp { get; set; }
} It fails. |
Minimal APIs needs to be configured with a different mechanism to customise the options compared to MVC. When updating to preview 7 I had to make the change for concrete types over interfaces as in this commit to get serialisation with Minima APIs working again: martincostello/apple-fitness-workout-mapper@3f13e70 |
@halter73 It seems our changes to make JSON writing more polymorphic has broken the source generator really badly 😄. I get an error like this with the above sample:
|
The problem is that the changes we made to make JSON writing more polymorphic only applied to objects returned via Task or ValueTask. Directly returned values were serialized polymorphically in .NET 6. See #39856 Same goes for already-existing overloads of Results.Ok(), Results.Json(), etc... We had to make the new generic overloads serialize polymorphically as well for compatiblity. aspnetcore/src/Http/Http.Results/src/HttpResultsHelper.cs Lines 44 to 47 in 4e0d4c4
This also goes for TypedResults. Although we could change the behavior of these without breaking existing apps, we didn't want a subtle behavioral difference between Results and TypedResults. |
@eiriktsarpalis Do you have any idea why we would have started seeing this between rc1 and rc2? |
I did an investigation, and it seems to be related to this change dotnet/runtime@f03470b#diff-65021de09c4fb9b39607f1f3ed228e53cd04b483880f66b0ea56310f71a5e400 from July but I can only find in RC2. @eiriktsarpalis @dotnet/area-system-text-json Is this related? |
Also, I confirmed that we could fix it using the non-generic overload and passing the runtime type directly. @dotnet/area-system-text-json do you have any other suggestions since that is exactly what you were doing before and changed in the commit I mentioned? |
Yes, it's related to this intentional breaking change: dotnet/docs#30758
That change did make it to RC1, perhaps it's not related to the issue at hand?
Explicitly passing the runtime type is the suggested workaround for the change. Out of curiosity, are you using a custom |
Interesting, maybe the RC1 SDK version installed in my machine is not the official one. I will use an official build to check if the change is already there. |
No, but we are calling |
dotnet/docs#30758 should only concern scenaria where a custom |
@eiriktsarpalis Here is a minimal repro: using System.Text.Json;
using System.Text.Json.Serialization;
var options = new JsonSerializerOptions();
options.AddContext<JsonContext>();
BaseClass obj = new MyClass() { BaseProperty = 10, MyProp = 1 };
Console.WriteLine(JsonSerializer.Serialize<object>(obj, options));
[JsonSerializable(typeof(MyClass))]
[JsonSerializable(typeof(BaseClass))]
public partial class JsonContext : JsonSerializerContext
{}
public class BaseClass
{
public int BaseProperty { get; set; }
}
public class MyClass : BaseClass
{
public int MyProp { get; set; }
} |
I can fix the issue with: -Console.WriteLine(JsonSerializer.Serialize<object>(obj, options));
+Console.WriteLine(JsonSerializer.Serialize(obj, obj.GetType(), options)); |
Also, this change will fix as well but cause the serialization issue mentioned here #41724 (comment): -Console.WriteLine(JsonSerializer.Serialize<object>(obj, options));
+Console.WriteLine(JsonSerializer.Serialize<BaseClass>(obj, options));
// or implict version: +Console.WriteLine(JsonSerializer.Serialize(obj, options)); |
Thanks. I can reproduce the failure in both RC1 and RC2. This is happening because of a combination of dotnet/docs#30755 and dotnet/docs#30758. The contract resolver must access the converter for A simple workaround would be to add an explicit var options = new JsonSerializerOptions();
options.AddContext<JsonContext>();
BaseClass obj = new MyClass() { BaseProperty = 10, MyProp = 1 };
Console.WriteLine(JsonSerializer.Serialize<object>(obj, options));
[JsonSerializable(typeof(object))]
[JsonSerializable(typeof(MyClass))]
[JsonSerializable(typeof(BaseClass))]
public partial class JsonContext : JsonSerializerContext
{ }
public class BaseClass
{
public int BaseProperty { get; set; }
}
public class MyClass : BaseClass
{
public int MyProp { get; set; }
} Related to dotnet/runtime#58134. Needing to explicitly specify all runtime types in polymorphic source generated scenaria is a known issue but needing to do it for |
Thanks for the clarification.
Without changes, in aspnetcore aspnetcore/src/Http/Http.Results/src/HttpResultsHelper.cs Lines 44 to 47 in 4e0d4c4
anyone using a |
This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes. See our Issue Management Policies for more information. |
Removing labels so the bot doesn't auto close this again before #43894 is merged. |
Is there an existing issue for this?
Describe the bug
This was a scenario that was working in .NET 6.0.
When using the .NET 7.0 System.Text.Json source generator with controllers (this is not reproduceable using minimal APIs), ASP NET Core fails to deserialize IEnumerable results.
Calling
JsonSerializer.Serialize(enumerable, JsonContext.Default.IEnumerableJsonTypeInfo)
manually succeeds.Expected Behavior
JSON serialization by the ASP NET Core controller infrastructure has the same behaviour as if
JsonSerializer.Serialize
was called.Steps To Reproduce
https://github.com/JakeYallop/JsonSourceGeneratorIEnumerableRepro
Exceptions (if any)
See above
.NET Version
7.0.100-rc.2.22459.2
Anything else?
dotnet --info output (click to view)
``` .NET SDK: Version: 7.0.100-rc.2.22459.2 Commit: 11f6b8f712Runtime Environment:
OS Name: Windows
OS Version: 10.0.19043
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\7.0.100-rc.2.22459.2\
Host:
Version: 7.0.0-rc.2.22451.11
Architecture: x64
Commit: 6d10e4c8bc
.NET SDKs installed:
3.1.422 [C:\Program Files\dotnet\sdk]
5.0.100 [C:\Program Files\dotnet\sdk]
5.0.104 [C:\Program Files\dotnet\sdk]
5.0.214 [C:\Program Files\dotnet\sdk]
5.0.303 [C:\Program Files\dotnet\sdk]
6.0.101 [C:\Program Files\dotnet\sdk]
6.0.108 [C:\Program Files\dotnet\sdk]
6.0.202 [C:\Program Files\dotnet\sdk]
6.0.203 [C:\Program Files\dotnet\sdk]
6.0.400 [C:\Program Files\dotnet\sdk]
7.0.100-rc.2.22459.2 [C:\Program Files\dotnet\sdk]
.NET runtimes installed:
Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.28 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.0-rc.2.22452.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.28 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.0-rc.2.22451.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 3.1.28 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.0-rc.2.22431.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Other architectures found:
arm64 [C:\Program Files\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\arm64\InstallLocation]
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]
Environment variables:
Not set
global.json file:
C:\Users\Agmas\source\repos\JsonSourceGenIEnumerableRepro\global.json
Learn more:
https://aka.ms/dotnet/info
Download .NET:
https://aka.ms/dotnet/download
Environment variables:
Not set
global.json file:
Not found
Learn more:
https://aka.ms/dotnet/info
Download .NET:
https://aka.ms/dotnet/download
The text was updated successfully, but these errors were encountered: