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

bug in F# discriminated union serialization #1126

Closed
quant1729 opened this issue Jul 7, 2015 · 14 comments
Closed

bug in F# discriminated union serialization #1126

quant1729 opened this issue Jul 7, 2015 · 14 comments

Comments

@quant1729
Copy link

Hello,

I have decided to give akka.net a try and unfortunately I faced a bug in 30 minutes after my start. The following my unit test crashes on my PC with the latest source code:

type testType1 = 
    string * string

type testType2 = 
    | V2 of testType1

[<Fact>]
let MyTest () =
    let x = V2("hello!","world")
    use sys = System.create "system" (Configuration.defaultConfig())
    let serializer = sys.Serialization.FindSerializerFor x
    let bytes = serializer.ToBinary x
    let des = serializer.FromBinary (bytes, typeof<testType2>) :?> testType2
    des
    |> equals x

Stack:

Test Outcome: Failed
Test Duration: 0:00:00.471

Result Message: System.NullReferenceException : Object reference not set to an instance of an object.

Trace:
at Akka.Serialization.NewtonSoftJsonSerializer.TranslateSurrogate(Object deserializedValue, ActorSystem system, Type type) in Akka\Serialization\NewtonSoftJsonSerializer.cs:line 132
at Akka.Serialization.NewtonSoftJsonSerializer.FromBinary(Byte[] bytes, Type type) in Akka\Serialization\NewtonSoftJsonSerializer.cs:line 114
at Akka.FSharp.Tests.ApiTests.MyTest() in Akka\src\core\Akka.FSharp.Tests\ApiTests.fs:line 76

I see other unit tests implemented to test this feature. they work ok. But they obviously dont test it thoroughly. Not sure how to fix the bug myself. I really appreciate your quick help.

Best regards,
Pavel.

@rogeralsing
Copy link
Contributor

Yes, this is a known issue, Json.NET does not play well with DU's in many cases.
We still have no good workaround for it as Json.NET does not emit type information for DU's.

Related to:
#999 #737

@rogeralsing
Copy link
Contributor

So, what does this actually compile into?
cc @Horusiath

type testType1 = 
    string * string

type testType2 = 
    | V2 of testType1

Other DU's contains nested types for each entry in the DU, which allows us to do duType.GetNestedTypes() to get the possible types for the DU.
But in this case, there are no nested types, and the DU type itself does not have any ctor, neither public or private.

So what is V2 behind the scenes?

@rogeralsing
Copy link
Contributor

NVM, I figured it out, I submitted a PR for this here #1127

@quant1729
Copy link
Author

Sounds strange as Json.net 7.0 works well with F# types. I think this issue is not related to it. I tried to run e.g. this piece of code:

[<Fact>]
let testJsonNet () =
    let x = V2("hello!","world")
    let strings = JsonConvert.SerializeObject x
    let des = JsonConvert.DeserializeObject<testType2> strings
    des
    |> equals x

Works fine for me.

@rogeralsing
Copy link
Contributor

We are not yet on 7.0

@quant1729
Copy link
Author

Again strange.. I took your code today. Compiled. I see nuget have installed Newtonsoft.Json.7.0.1 on my PC.

@quant1729
Copy link
Author

Sorry, I am wrong. Indeed it is ver 6.0. Can you please tell me what are the plans regarding the upgrade to version 7.0?

@rogeralsing
Copy link
Contributor

Yes, we have talked about it, but not decided when it should be done.
Json.NET 7 solves some issues but not all, e.g. deserializing object arrays containing DU's.
And we have not tested how it plays with our own serializer logic as we do quite a bit of special handing for things like ActorRef's and other special message types.

We need a bit of time to evaluate how it works out

@quant1729
Copy link
Author

I see. Looking forward to your upgrade. Meanwhile I will serialize DUs myself using version 7.0 and try to send strings between actors.

@rogeralsing
Copy link
Contributor

Im trying Json.NET right now. is there anything special one need to do when using v7 and DU's?
I'm not seeing the fix, even if I remove all our own custom settings to the serializer.

@rogeralsing
Copy link
Contributor

[edit] I'm a donkey..

@rogeralsing rogeralsing mentioned this issue Jul 7, 2015
@rogeralsing
Copy link
Contributor

I was wrong, and Json.net yet again handles everything we throw at it..
By creating a JsonSerializer from the JsonSerializationSettings, we could then pass that into ToObject and everything magically works..
All is good :)

@quant1729
Copy link
Author

Yes, DU are not the type in usual OO sense, but a hierarchy of classes that implement this algebraic type.

@rogeralsing
Copy link
Contributor

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants