Bug in Json.Net: System.Int32 is deserialized as System.Int64

[ukluk]

My state object produces errors during serialization in the Boolean object. Do I need to update to 1.1.1?

This is the object:

[JsonObject(MemberSerialization.OptIn)]
public class SiteState : global::Orleans.GrainState
{
    [JsonProperty]
    public AsSite Model { get; set; }

    //defaults
    [JsonProperty]
    public int Threshold { get; set; }

    [JsonProperty]
    public bool DisableAlarms { get; set; }

}

and this is the error:

Data #1 Key=Model Value=Mesh.Ratatouille.Model.Site.AsSite Type=Mesh.Ratatouille.Model.Site.AsSite
Data #2 Key=Threshold Value=0 Type=System.Int64
Data #3 Key=DisableAlarms Value=False Type=System.Boolean
at Orleans.Storage.AzureTableStorage.ConvertFromStorageFormat(GrainState grainState, GrainStateEntity entity)
at Orleans.Storage.AzureTableStorage.d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Scheduler.SchedulerExtensions.<>c__DisplayClassa.<b__8>d__c.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__35.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__2b.MoveNext()
Exc level 1: System.ArgumentException: Object of type 'System.Int64' cannot be converted to type 'System.Int32'.
at System.RuntimeType.TryChangeType(Object value, Binder binder, CultureInfo culture, Boolean needsSpecialCast)
at System.RuntimeType.CheckValue(Object value, Binder binder, CultureInfo culture, BindingFlags invokeAttr)
at System.Reflection.MethodBase.CheckArguments(Object[] parameters, Binder binder, BindingFlags invokeAttr, CultureInfo culture, Signature sig)
at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at System.Reflection.RuntimePropertyInfo.SetValue(Object obj, Object value, Object[] index)
at Orleans.GrainState.SetAll(IDictionary`2 values)
at Orleans.Storage.AzureTableStorage.ConvertFromStorageFormat(GrainState grainState, GrainStateEntity entity)

[gabikliot]

I suspect it is the same issue we had with Guid with Json serializer: #1187 (comment) and JamesNK/Newtonsoft.Json#762 and JamesNK/Newtonsoft.Json#728 (comment).

This is, as far as I am concerned, a bug in Json.Net. They consider it a feature. Essentially, they loose type info for primitives and some known types, like Guid, which have a built in conversion to strings. When read back they sometimes don't consider the original type that was used, and in your case the default deserialzition of a number is back to long and not int. We had this issue with long in the past: #1047 (comment).

[ukluk]

Gabi thanks,

I look briefly into the other issue and understand the problem, but other than taking a external serializer, is there a way to decorate the pocos?

[gabikliot]

Try long instead of int in your poco.

[Allon-Guralnek]

@gabikliot

There are additional gotcha in this area that I discovered when deserializing using Json.NET with weak typing (i.e. when Json.NET doesn't have specific type information during deserialization):

1. As you've discovered, all numbers are deserialized as long. Well, almost all numbers. If you deserialize a UInt64 with a value larger than Int64.MaxValue, it will deserialize into a BigInteger instead of a UInt64.
2. You've also discovered that Guid will be deserialized as String.
3. System.Single is deserialized into System.Double unless specified otherwise by JsonSerializerSettings.FloatParseHandling.
4. Enums are serialized as numbers, so they will be deserialized as Int64, unless StringEnumConverter was used for serialization, then it will be deserialized as a String. In any case, conversion should be tolerant to both.

Due to all this, I've written my own conversion code to fix any loss of type information when converting to JSON, included below. It's not perfect, and it evolves as I discover new corner cases, but it addresses some of the oddities you might encounter.

public static class JsonHelper
{
    private static readonly Type[] _specialNumericTypes = { typeof(ulong), typeof(uint), typeof(ushort), typeof(sbyte) };

    /// <summary>
    /// Converts values that were deserialized from JSON with weak typing (e.g. into <see cref="object"/>) back into
    /// their strong type, according to the specified target type.
    /// </summary>
    /// <param name="value">The value to convert.</param>
    /// <param name="targetType">The type the value should be converted into.</param>
    /// <returns>The converted value.</returns>
    public static object ConvertWeaklyTypedValue(object value, Type targetType)
    {
        if (targetType == null)
            throw new ArgumentNullException(nameof(targetType));

        if (value == null)
            return null;

        if (targetType.IsInstanceOfType(value))
            return value;

        var paramType = Nullable.GetUnderlyingType(targetType) ?? targetType;

        if (paramType.IsEnum)
        {
            if (value is string)
                return Enum.Parse(paramType, (string)value);
            else
                return Enum.ToObject(paramType, value);
        }

        if (paramType == typeof(Guid))
        {
            return Guid.Parse((string)value);
        }

        if (_specialNumericTypes.Contains(paramType))
        {
            if (value is BigInteger)
                return (ulong)(BigInteger)value;
            else
                return Convert.ChangeType(value, paramType);
        }

        if (value is long)
        {
            return Convert.ChangeType(value, paramType);
        }

        throw new ArgumentException($"Cannot convert a value of type {value.GetType()} to {targetType}.");
    }
}

[gabikliot]

Thank you Alon!
And how do you hook it up to Json setting/converter/serialziler?

[ukluk]

Alon,

The variable that broke for me was a bool. _Nice if you could add also a condition to parse true/false._

Yet it is confusing because there are other bool properties in the state that I am serializing and there is no exception.

This is the exception that crashed on type Boolean:

Data #3 Key=DisableAlarms Value=False Type=System.Boolean
at Orleans.Storage.AzureTableStorage.ConvertFromStorageFormat(GrainState grainState, GrainStateEntity entity)
at Orleans.Storage.AzureTableStorage.d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Scheduler.SchedulerExtensions.<>c__DisplayClassa.<b__8>d__c.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__35.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__2b.MoveNext()
Exc level 1: System.ArgumentException: Object of type 'System.Int64' cannot be converted to type 'System.Int32'.

[ukluk]

Gabi, Alon,

My 2 cents:
Why is the method trying to serialize to/from "object" if your state knows it's type?
If the function was ReadStateAsync, WriteStateAsync then everything works as the map is _explicit._

I did a simple test bypassing the internal state management,
OnActivate I manually call TableStorage Read and I use JsonConverter with strong typing. It never fails.

it will require a lot of refactoring, but pass the template to this function:

    /// <summary>
    /// Deserialize from Azure storage format
    /// </summary>
    /// <param name="entity">The Azure table entity the stored data</param>
    internal object ConvertFromStorageFormat(T dataState, GrainStateEntity entity)
    {
         try
        {
            if (entity.Data != null)
            {
                // Rehydrate
                dataState = SerializationManager.DeserializeFromByteArray<T>(entity.Data);
            }
            else if (entity.StringData != null)
            {
                dataState = Newtonsoft.Json.JsonConvert.DeserializeObject<T>(entity.StringData, jsonSettings);
            } 

            // Else, no data found
        }

or somewhere in the call pass the type, then the Json corner cases disappear.

[Allon-Guralnek]

@gabikliot: I don't hook it up to Json.NET itself. Since I mostly deserialize into strongly-typed classes, and only in a few specific areas I deal with weakly typed data (such as something that comes from reflection), I simply call this method for every value that was weakly typed. Hooking up to Json.NET could incur a performance penalty for strongly typed data that doesn't need such 'fixups'.

@ukluk: I'm not sure how it could break for a bool, since it has a unique representation in JSON, therefore you never lose any type information. Could you show an example?

Also, while all these issues can indeed be avoided if you deserialize into a strongly typed object (which I think is what Orleans should aspire to do) sometimes you don't have the type information to that, or its too complicated to do it (e.g. runtime codegen).

[ukluk]

@Allon-Guralnek I included the exception, but here the bomb (Data #3):

Data #3 Key=DisableAlarms Value=False Type=System.Boolean
at Orleans.Storage.AzureTableStorage.ConvertFromStorageFormat(GrainState grainState, GrainStateEntity entity)
at Orleans.Storage.AzureTableStorage.d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Scheduler.SchedulerExtensions.<>c__DisplayClassa.d__c.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__35.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
at Orleans.Runtime.Catalog.d__2b.MoveNext()
Exc level 1: System.ArgumentException: Object of type 'System.Int64' cannot be converted to type 'System.Int32'.

[gabikliot]

@ukluk , can you try to define that class without extending GrainState.
We now support POCO state classes. I wonder if that will help.
So try just plain: public class SiteState.
You might need to update to latest master version for that.

[gabikliot]

Thank you @Allon-Guralnek . I had exactly the same impersonation, after playing with Json.Net, like you wrote:

"Also, while all these issues can indeed be avoided if you deserialize into a strongly typed object (which I think is what Orleans should aspire to do) sometimes you don't have the type information to that, or its too complicated to do it (e.g. runtime codegen)."

Basically, it is almost impossible, as far as I can tell, to write a fully 100% compatible Json.Net serialier that will take ANY dynamic type and will correctly deserialize it back. Json.Net simply looses some dynamic type info. For example, for Dictionary<string, object> when values are of different types, it is basically impossible to deserialize it back correctly. This is since the serialized Dictionary (the end string) simply does not include the dynamic type of the object values. And this is "by design".

Now, I was inaccurately saying it is not possible. It is possible, if one writes basically his full Converter to correctly handle ALL types in all situations. Basically, you end up building a "serializer within the serializer" - Json.Net will be calling your methods and you will be dealing with emitting the right type info into strings and on the reverse path it will be your code to correctly reconstruct the type back. Basically, for us, Orleans, that would look pretty similar to our binary serializer that we have now, just emitting strings.

I don't think we want to go that route, at least not in the near future.

Instead, what we should try to do, as much as we can, is what you both wrote:
Try to preserve the original type and not cast to Object.
Specifically, in the storage provider, we should try not to cast the Grain State POCO to Object.
I think with recent introduction of POCO grain state in #1060, it will make this easier.
But I am afraid we still cast the POCO state object to object, since the interface of the storage provider does not currently expose generic state T state.
public async Task WriteStateAsync(string grainType, GrainReference grainReference, IGrainState grainState)
does not accept T but rather IGrainState has object State property.
So maybe the solution is pump T all the way down, basicaly like @ukluk suggested: ConvertToStorageFormat(T dataState, GrainStateEntity entity) instead of currently ConvertToStorageFormat(object grainState, GrainStateEntity entity)

Or maybe there is another way around it.

[Allon-Guralnek]

@ukluk: Sorry, I still don't understand the issue. I'm talking pure Json.NET, not Orleans.

@gabikliot: There is no need to pump T all the way down for POCOs since Json.NET can include type information in the JSON itself (with TypeNameHandling.Auto). This will ensure a POCO cast to object then serialized to JSON then deserialized as an object will be deserialized correctly. It is only when serializing certain "loose primitive" types that are not attached to any defined class, such as inside an object[] or Dictionary<string, object> is there a problem, because the type information is not serialized with those types, and thus not available to Json.NET, it only knows it needs to make an object.

In my case, I have an RPC mechanism over JSON and I need to serialize the arguments to a method. When it serializes an object[] with those arguments, if all arguments are user-defined classes, great, since they all include $type fields. But if someone tries to pass a loose int outside of a class, then it will materialize as long on the other side.

What I would love to so is at compile time generate a class per method that will have a strongly-typed property for each parameter of a given method, then fill and serialize that class, and not an object[]. That will ensure perfect deserialization on the other side.

I cannot do that since I don't have a good code generation infrastructure. Orleans of the other hand, does, and should do that. You shouldn't use a Dictionary<string, object>, you should either serialize the original class (and not decompose it into a weakly-typed data structure) or if there is no such class, generate a strongly-typed class to hold that data.

If you do that, you will never encounter any of Json.NET's serialization oddities.

[gabikliot]

@Allon-Guralnek, ohhh, maybe we need to try TypeNameHandling.Auto instead of TypeNameHandling.All that we use now? Will try later. Thanks!

The full solution is yes - reuse Orleans code gen, to code gen fully correct Json serializer (to solve the Dictionary<string, object> problem and other loose cases like you wrote). That is what I wrote above: building a "serializer within the serializer" .
Yes, makes me very excited to go and explain to people how Json.Net is not always working and how we fixed it. I just don't think we want to be in that business. I think we are here for Distributed Programming Models, not for fixing other people's broken serializers. But technically, yes, it should be possible. I just don't think we want to spend time on that.

[gabikliot]

Maybe @ReubenBond wants to chime in here, w.r.t. to @Allon-Guralnek 's suggestion on using Orleans code gen to solve "Json.NET's serialization oddities". I should not be saying so categorically that "we don't want to spend time on that". I know I don't. But maybe other do.

[Allon-Guralnek]

@gabikliot: TypeNameHandling.Auto won't help you if TypeNameHandling.All didn't work for you, since Auto includes less type information (it includes type information only when polymorphism is detected).

Json.NET uses a wire format it has no control over, therefore it works for JSON-compatible data types or when it has type information. When you use types that are represented differently in JSON and also you don't give it any type information, only then it falters. I don't think I'd call that 'broken' as it never failed to provide what it claims. You simply have more advanced requirements, but then you can consider Orleans serializes to be "broken" since they don't have a widely-compatible and universally-readable wire format (again, not really broken since it was never a requirement Orleans serializers tried to fulfill).

[gabikliot]

Thanks for explaining that Allon. I did not know it was because of the standard wire compatibility requirements. Makes sense now.

[gabikliot]

As was discussed here and in other related issues, this is not a bug in Orleans, but is rather the way Json serializer works.
Related issue is #1495, which ensures Orleans properly supports JSON serializer.

Closing this issue.