System.Text.Json Serialization Regression in NET6 (Net5 worked)

[jogibear9988]

Description

I Serialize a Class with an IQueryable in it, looks like this:

public class WebsocketResponse {
public object Response {get;set;}
}

Reproduction Steps

Test Project:

use following 2 nugets:

<PackageReference Include="linq2db" Version="3.5.1" />
<PackageReference Include="Microsoft.Data.Sqlite.Core" Version="5.0.11" />

and following code:

      using LinqToDB.Data;
      using System.IO;
      using System.Text.Json;
      
      using (var dc = new DataConnection("Sqlite", "Data Source=:memory:"))
      using (var ms = new MemoryStream())
      {
          using (var writer = new Utf8JsonWriter(ms))
          {
              JsonSerializer.Serialize(writer, new Response { Result = dc.GetTable<DbTable>() });
          }
      }
      
      public class DbTable
      {
          public string A { get; set; }
      }
      
      public class Response
      {
          public object Result { get; set; }
      }

Expected behavior

Serialization should work as it did before.

Actual behavior

System.NotSupportedException: 'The type 'LinqToDB.Linq.Table`1[...]' can only be serialized using async serialization methods.'

Regression?

Yes, this did work in Net5

Known Workarounds

No response

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

I Serialize a Class with an IQueryable in it, looks like this:

public class WebsocketResponse {
public object Response {get;set;}
}

Reproduction Steps

Test Project:

use following 2 nugets:

<PackageReference Include="linq2db" Version="3.5.1" />
<PackageReference Include="Microsoft.Data.Sqlite.Core" Version="5.0.11" />

and following code:

    using LinqToDB.Data;
    using System.Text.Json;
    
    using (var dc = new DataConnection("Sqlite", "Data Source=:memory:;Version=3;New=True;"))
    using (var ms = new MemoryStream())
    {
        using (var writer = new Utf8JsonWriter(ms))
        {
            JsonSerializer.Serialize(writer, new Response { Result = dc.GetTable<DbTable>() });
        }
    }
    
    public class DbTable
    {
        public string A { get; set; }
    }
    
    public class Response
    {
        public object Result { get; set; }
    }

Expected behavior

Serialization should work as it did before.

Actual behavior

System.NotSupportedException: 'The type 'LinqToDB.Linq.Table`1[...]' can only be serialized using async serialization methods.'

Regression?

Yes, this did work in Net5

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	jogibear9988
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[jogibear9988]

If you run the example code in net5 it will work. (it will still throw an exception, but you see that it trys to enumerate the IQueryable)

[jogibear9988]

A workaround I could use atm:

    public static IEnumerable AsUntypedEnumerable(this IQueryable query)
    {
        foreach(var o in query)
            yield return o;
    }

[huoyaoyuan]

The error message looks related to IAsyncEnumerable.

[eiriktsarpalis]

Agree with @huoyaoyuan's assessment. Presumably this happens because the ITable<T> interface inherits from IAsyncEnumerable<T>. I would say that this is by design. Out of curiosity, what was the expected serialization here?

[huoyaoyuan]

@eiriktsarpalis Some libraries like EF treats IAsyncEnumerable<T> as an improvement - the collection can be iterated either synchronise or asynchronise. Refuse the synchronise version if the asynchronise is available looks breaking.

[eiriktsarpalis]

Hmm. IAsyncEnumerable does take precedence over IEnumerable since it is assumed that types implementing both interfaces will block when iterating as IEnumerable<T>.

At the same time the IAsyncEnumerable converter statically has no visibility over whether IEnumerable<T> is implemented, as such it will (rightly) fail the serialization if attempting to serialize via the sync serialization APIs.

I think the best solution in the future might be to expose IEnumerable converter factory so that users are able to register that behavior for the types they need. A similar request has been filed for users needing to opt into the object converter for classes implementing IEnumerable.

[jogibear9988]

That all means the breaking change will stay as is in Net6?

But why does System.Text.Json use IAsyncEnumerable at all, in the synchronous serialization methods, when it can only serialize with async serialization methods?

[jogibear9988]

[jogibear9988]

I think System.Text.json should, when I use the synchronous methods, stop using the IAsyncEnumerable interface.
If I use SerializeAsync then it should, but then it also would not break.

[eiriktsarpalis]

I think System.Text.json should, when I use the synchronous methods, stop using the IAsyncEnumerable interface.

Unfortunately that is not technically possible with the current converter model. Each converted type/member is fixed to a specific converter instance (and from the converter's perspective it's serializing a TCollection : IAsyncEnumerable<T> with no visibility on whether TCollection is also IEnumerable<T>). I should also point out that dynamically falling back to IEnumerable<T> if using a sync method has the potential of producing inconsistent outputs depending on whether you're serializing synchronously or asynchronously (since there's no guarantee that the implementations produce identical elements).

The only real workaround is to force blocking enumeration of the IAsyncEnumerable<T> (e.g. via #60363), but again this is something we would like to avoid unless the user opted in to that. It also wouldn't work on the browser.

[eiriktsarpalis]

FWIW this is documented in a relevant breaking change document: https://docs.microsoft.com/en-us/dotnet/core/compatibility/serialization/6.0/iasyncenumerable-serialization#new-behavior

[jogibear9988]

in the breaking change you list that the async methods will enumerate the iasyncenumerable, not that the synchronous will fail.

The only real workaround is to force blocking enumeration of the IAsyncEnumerable, but again this is something we would like to avoid unless the user opted in to that.
==> how do I opt into this?

[eiriktsarpalis]

how do I opt into this?

Here's how I would write it. Be warned that a) this hasn't been tested and b) it forces blocking enumeration in the async case as well:

public class BlockingAsyncEnumerableConverter : JsonConverterFactory
{
    public override bool CanConvert(Type typeToConvert) => ImplementsAsyncEnumerable(typeToConvert, out _);

    public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        if (ImplementsAsyncEnumerable(typeToConvert, out Type? elementType))
        {
            Type converterType = typeof(BlockingAsyncEnumerableConverter<,>).MakeGenericType(typeToConvert, elementType);
            return (JsonConverter)Activator.CreateInstance(converterType)!;
        }

        throw new ArgumentException(nameof(typeToConvert));
    }

    private static bool ImplementsAsyncEnumerable(Type type, [NotNullWhen(true)] out Type? elementType)
    {
        if (type.IsInterface && IsAsyncEnumerable(type, out elementType))
        {
            return true;
        }

        foreach (Type interfaceTy in type.GetInterfaces())
        {
            if (IsAsyncEnumerable(type, out elementType))
            {
                return true;
            }
        }

        elementType = null;
        return false;

        static bool IsAsyncEnumerable(Type type, [NotNullWhen(true)] out Type? elementType)
        {
            if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>))
            {
                elementType = type.GetGenericArguments()[0];
                return true;
            }

            elementType = null;
            return false;
        }
    }
}

public sealed class BlockingAsyncEnumerableConverter<TCollection, TElement> : JsonConverter<TCollection>
    where TCollection : IAsyncEnumerable<TElement>
{
    private readonly static JsonConverter<TCollection> s_defaultConverter = (JsonConverter<TCollection>)new JsonSerializerOptions().GetConverter(typeof(TCollection));
    private JsonConverter<TElement>? _elementConverter;

    public override TCollection? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return s_defaultConverter.Read(ref reader, typeToConvert, options);
    }

    public override void Write(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options)
    {
        var elementConverter = _elementConverter ??= (JsonConverter<TElement>)options.GetConverter(typeof(TElement));
        var enumerator = value.GetAsyncEnumerator();

        try
        {
            writer.WriteStartArray();

            while (enumerator.MoveNextAsync().ConfigureAwait(false).GetAwaiter().GetResult())
            {
                elementConverter.Write(writer, enumerator.Current, options);
            }

            writer.WriteEndArray();
        }
        finally
        {
            enumerator.DisposeAsync().ConfigureAwait(false).GetAwaiter().GetResult();
        }
    }
}

[eiriktsarpalis]

Related to #1808.

[eiriktsarpalis]

Triage: issue can be worked around by either casting the value to IEnumerable or registering a custom converter. In the future we should explore adding additional APIs that let users opt-in to internal converters that are not resolved by default for the given type (cf. #1808).

Moving to 7.0.0

[jogibear9988]

I think the breaking changes need to be fixed, so people also know that the Synchronous methods are also changed.

And I think I tried to use AsEnumerable(), but that did not work (but I have to check again)

[mwwhited]

Before System.Text.Json was updated with nullable enabled it was possible to use JsonSerializer.Serialize(obj) where obj was just an interface type. (This is apparent by trying to serialize any dispatch proxy). If you try to use JsonSerializer.Serialize(...) against any dispatch proxy you now get a null reference exception from the OrdinalIgnoreCaseComparer.GetHashCode(String obj). You can still serialize these runtime generated classes in .Net Core 3.1 but they fail in the latest versions of .Net 5.0 and .Net 6.0.

` System.ArgumentNullException: Value cannot be null. (Parameter 'obj')

Stack Trace: 
OrdinalIgnoreCaseComparer.GetHashCode(String obj)
ParameterLookupKey.GetHashCode()
Dictionary2.FindValue(TKey key) Dictionary2.TryGetValue(TKey key, TValue& value)
JsonTypeInfo.InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, Boolean sourceGenMode)
JsonTypeInfo.ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
JsonSerializerOptions.g__CreateJsonTypeInfo|112_0(Type type, JsonSerializerOptions options)
JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
JsonSerializerOptions.GetOrAddClass(Type type)
JsonSerializer.GetTypeInfo(JsonSerializerOptions options, Type runtimeType)
JsonSerializer.Serialize(Object value, Type inputType, JsonSerializerOptions options)
`

You can work around this with a custom converter but it would be nice if System.Text.Json was returned to the original operation as the converter workaround only works if you use the generic version of the JsonSerializer.Serialize<T>(obj)

[eiriktsarpalis]

@mwwhited I might be misunderstanding your problem but it seems unrelated to the original issue. I would recommend creating a new issue preferably with a minimal reproduction so we can understand what the problem is.

[mwwhited]

It's the same fundamental issue. It was a breaking change introduced when you guys add all the null checks and additional abstractions in .Net 5 without sanity checking the changes to see if they changed conditional logic.

[eiriktsarpalis]

I'm not sure I see the connection. The original report specifically refers to IAsyncEnumerable serialization. I think a minimal repro in a separate issue would help. Thanks.

[eiriktsarpalis]

Closing in favor of #63791.