Combining 'JsonDerivedType' attribute with a custom 'JsonConverter' on the derived class throws NotSupportedException on deserialization

[ChrML]

Issue summary
The polymorphic JSON deserialization in System.Text.Json has some limitations when combined with custom JSON converters. This is documented here:

Polymorphism using type discriminators is only supported for type hierarchies that use the default converters for objects, collections, and dictionary types.

https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism?pivots=dotnet-7-0#additional-polymorphic-serialization-details

I'm creating an issue here because I can't see any technical limitations of why this shouldn't be possible. Supporting this shouldn't require more read-ahead or buffering than not supporting it. Supporting such use cases would be extremely useful for a scenario I have right now where I need a custom JSON converter for doing the type-discrimitation instead of relying on the build-in feature for this, because one of the derived types has a custom JSON converter.

I also assume this causes similar issues with JsonConverter's generated by source-generators. Which basically makes the built-in type-discrimination not combinable with the JSON source generators.

Example code

using System.Text.Json;
using System.Text.Json.Serialization;
using Xunit;

namespace Example;

[JsonDerivedType(typeof(DerivedClass), "Derived")]
public class BaseClass
{
}

[JsonConverter(typeof(DerivedClassConverter))]    // Comment out this line to make the test run.
public class DerivedClass: BaseClass
{
}

public class DerivedClassConverter: JsonConverter<DerivedClass>
{
    public override DerivedClass? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        reader.Skip();
        return new DerivedClass();
    }

    public override void Write(Utf8JsonWriter writer, DerivedClass value, JsonSerializerOptions options) => throw new NotImplementedException();
}

public class TestRun
{
    [Fact]
    public void Test()
    {
        // Works.
        DerivedClass? obj1 = JsonSerializer.Deserialize<DerivedClass>("{\"$type\":\"Derived\"}");

        // NotSupportedException: The converter for derived type 'Example.DerivedClass' does not support metadata writes or reads.
        BaseClass? obj2 = JsonSerializer.Deserialize<BaseClass>("{\"$type\":\"Derived\"}");
        Assert.IsType<DerivedClass>(obj2);
    }
}

Throws:

Message: 
System.NotSupportedException : The converter for derived type 'Example.DerivedClass' does not support metadata writes or reads. Path: $ | LineNumber: 0 | BytePositionInLine: 19.
---- System.NotSupportedException : The converter for derived type 'Example.DerivedClass' does not support metadata writes or reads.

  Stack Trace: 
ThrowHelper.ThrowNotSupportedException(ReadStack& state, Utf8JsonReader& reader, NotSupportedException ex)
JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan`1 json, JsonTypeInfo jsonTypeInfo)
JsonSerializer.Deserialize[TValue](String json, JsonSerializerOptions options)
TestRun.Test() line 49

The exception doesn't give a good indication of what the problem is, so it took some time searching the .NET source-code to find the actual cause the exception. Originally thrown from here:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.MetadataHandling.cs

Line 38 in 41772ba

ThrowHelper.ThrowNotSupportedException_DerivedConverterDoesNotSupportMetadata(resolvedType.Type);

   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_DerivedConverterDoesNotSupportMetadata(Type derivedType)
   at System.Text.Json.Serialization.JsonConverter.ResolvePolymorphicConverter(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)

[eiriktsarpalis]

I'm creating an issue here because I can't see any technical limitations of why this shouldn't be possible.

The answer is in the exception message really. Polymorphic deserialization requires requires using the built-in converters that are capable of reading polymorphism metadata, and in your case being able to resume deserialization after the base converter has read metadata. By swapping out the built-in converter with a custom one your type loses the ability to resume deserialization.

[ChrML]

I will look into this on monday. It sounds more like a limitation of the current implementation itself than a fundamental issue (would be sub-optimal/unfeasible/impossible), or am I missing something?

I had no issues writing my own JsonConverter for the base class that assigns/clones the Utf8JsonReader struct to read the first $type property, and then resumes deserialization from the beginning of the object on the derived type using the original reader (regardsless if the derived type has a custom or built-in converter).

Since the derived type converter reads the object fully, and creates the deserialized object, I'm uncertain what more metadata is needed.

[eiriktsarpalis]

Unlike custom converters, the built-in converters need to take streaming deserialization into account. In other words, when streaming a large JSON object the Utf8JsonReader typically contains a rolling window of JSON data and not necessarily the entire contents necessary for a nested custom converter to resume deserialization.

In the future, it might be possible for us to detect derived types containing custom converters and simply buffer the entire JSON payload to support your scenario.

[ChrML]

I understand the need for built-in converters to be able to read partial objects from a stream of data.

However, a scenario with read-ahead (read until the current object is terminated, so the converter gets the full data of the current object) is already the practice for custom converters today. To lower the complexity of writing custom convers I guess, by sacrificing that possibility.

So basically it's the fact that there is polymorphic handling at the base type that ruins the scenario, because I guess it has already started started consuming the object in a resumable state. Which boils down to the implementation details internally in System.Text.Json because one has already accepted that for custom converters in general it's acceptable to buffer the full object before letting the converter work on the data. From a consumer of the library's perspective, this limitation doesn't make much sense though, since it would be technically the same computing to do so as for any other custom converter.

What I guess is there are a few possibilities upon detecting that the converter doesn't support continuation, and the current buffer doesn't terminate the object:

1. Grab a buffer from a shared pool, and read all the data into it to the object terminator (using the continuation logic already in place in multiple internal converters). Create a Utf8JsonReader on that buffer and pass it to the custom converter, then give it back to the pool.

2. Or, see if it's possible to trigger a read-ahead somehow from the current resumable state (without purging the type discriminator data that is already read), and after reading the full object, pass it to the custom converter from the beginning of it. Then purge/allow the read data to be purged. This would be most performance efficient.