Skip to content
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

JsonSerializer throw an InvalidCastException when serializing an array of item with a custom converter #46522

Closed
Tracked by #63918
meziantou opened this issue Jan 3, 2021 · 7 comments · Fixed by #73259
Assignees
Milestone

Comments

@meziantou
Copy link
Contributor

meziantou commented Jan 3, 2021

Description

JsonSerializer throw an InvalidCastException when serialization a collection of items. The class has a custom JsonConverter that matches one of the interface implemented by the class.

Serializing an SampleRepro[] (SampleRepro implements the interface) throw an InvalidCastException.

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>
</Project>
using System;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace ConsoleApp25
{
    class Program
    {
        static void Main()
        {
            // OK
            Console.WriteLine(JsonSerializer.Serialize(new SampleRepro()));

            // Exception
            Console.WriteLine(JsonSerializer.Serialize(new SampleRepro[] { new SampleRepro() }));
        }
    }

    public interface IRepro<T>
    {
        T Value { get; }
    }

    [JsonConverter(typeof(ReproJsonConverter))]
    public class SampleRepro : IRepro<object>
    {
        public object Value => "";
    }

    public sealed class ReproJsonConverter : JsonConverter<IRepro<object>>
    {
        public override bool CanConvert(Type typeToConvert)
        {
            return typeof(IRepro<object>).IsAssignableFrom(typeToConvert);
        }

        public override IRepro<object> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotSupportedException();

        public override void Write(Utf8JsonWriter writer, IRepro<object> value, JsonSerializerOptions options)
        {
            if (value is null)
            {
                writer.WriteNullValue();
            }
            else
            {
                JsonSerializer.Serialize(writer, value.Value, options);
            }
        }
    }
}
Unhandled exception. System.InvalidCastException: Unable to cast object of type 'ConsoleApp25.ReproJsonConverter' to type 'System.Text.Json.Serialization.JsonConverter`1[ConsoleApp25.SampleRepro]'.
   at System.Text.Json.Serialization.Converters.IEnumerableDefaultConverter`2.GetElementConverter(WriteStack& state)
   at System.Text.Json.Serialization.Converters.ArrayConverter`2.OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.Converters.IEnumerableDefaultConverter`2.OnTryWrite(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](JsonConverter jsonConverter, Utf8JsonWriter writer, TValue& value, JsonSerializerOptions options, WriteStack& state)
   at System.Text.Json.JsonSerializer.WriteCore[TValue](Utf8JsonWriter writer, TValue& value, Type inputType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue& value, Type inputType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at ConsoleApp25.Program.Main() in C:\Users\mezia\source\repos\ConsoleApp25\ConsoleApp25\Program.cs:line 16

Configuration

.NET 5.0.1

Regression?

This is new code, I haven't tested with previous versions of System.Text.Json

Other information

I managed to make it work using a JsonConverterFactory but I'm not sure this is needed or just a temporary workaround.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Jan 3, 2021
@huoyaoyuan
Copy link
Member

The exception message tells the problem: you must use JsonConverter<SampleRepro>, not JsonConverter<IRepro<object>>.

A JsonConverter<T> must be associated with a concrete type. Only JsonConverterFactory can be used for a family of types.

@meziantou
Copy link
Contributor Author

The way S.T.Json handle converters seems inconsistent:

  • When serializing a single value, it works with JsonConverter<IRepro<object>>. => According to your comment, this shouldn't be the case?
  • When serializing a collection, it throws an InvalidCastException. As a user, this exception seems like a bug in S.T.Json.
  • When serializing a single value with a non-compatible converter (CanConvert returns false), it throws an explicit exception "System.InvalidOperationException: The converter specified on 'ConsoleApp25.SampleRepro' is not compatible with the type 'ConsoleApp25.SampleRepro'." => This seems perfectly valid.

Maybe S.T.Json should throw an InvalidOperationException in this case instead of an InvalidCastException to make it clear that the converter is not the right way to go. But this is inconsistent with the first case that works.

@huoyaoyuan
Copy link
Member

  • When serializing a single value, it works with JsonConverter<IRepro<object>>.

It surprises me. I'd expect it not to work.

@meziantou
Copy link
Contributor Author

It surprises me. I'd expect it not to work.

You can run the code from the issue and check the result of the first serialization Console.WriteLine(JsonSerializer.Serialize(new SampleRepro()));. It seems to correctly calls the converter.

@layomia layomia self-assigned this Feb 3, 2021
@eiriktsarpalis
Copy link
Member

If anything, it is likely accidental that the first line happens to work. At the same time, the exception thrown in the second line is not very user friendly. We should consider adding checks that detect incompatible converter types early on and surface a relevant error message.

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 23, 2021
@eiriktsarpalis eiriktsarpalis added this to the 6.0.0 milestone Jul 23, 2021
@eiriktsarpalis
Copy link
Member

This is related to #55135.

@eiriktsarpalis
Copy link
Member

See #55135 (comment) for analysis of the issue and proposals on how to address this in the future.

@eiriktsarpalis eiriktsarpalis modified the milestones: 6.0.0, 7.0.0 Jul 30, 2021
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 10, 2022
@jeffhandley jeffhandley removed the enhancement Product code improvement that does NOT require public API changes/additions label Jul 10, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 2, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 7.0.0 Aug 2, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants