Zaid-Ajaj / Fable.Remoting

Type-safe communication layer (RPC-style) for F# featuring Fable and .NET Apps
https://zaid-ajaj.github.io/Fable.Remoting/
MIT License
272 stars 54 forks source link

A record field of type "string option" with a non-None value blows up Json deserialisation #274

Closed zelenij closed 2 years ago

zelenij commented 2 years ago

I have a record type with one of the fields defined as

PlaceId: string option

And when I set the value of the field to Some "value" on the client and then submit to the server, this is the exception I get:

 Newtonsoft.Json.JsonSerializationException: Cannot deserialize the current JSON object (e.g. {"name":"value"}) into type 'System.String' because the type requires a JSON primitive value (e.g. string, number, boolean, null) to deserialize correctly.
To fix this error either change the JSON to a JSON primitive value (e.g. string, number, boolean, null) or change the deserialized type so that it is a normal .NET type (e.g. not a primitive type like integer, not a collection type like an array or List<T>) that can be deserialized from a JSON object. JsonObjectAttribute can also be added to the type to force it to deserialize from a JSON object.
Path '[0].placeId.value'.
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.Serialization.JsonSerializerProxy.DeserializeInternal(JsonReader reader, Type objectType)
   at Fable.Remoting.Json.FableJsonConverter.ReadJson(JsonReader reader, Type t, Object existingValue, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonSerializer.Deserialize(JsonReader reader, Type objectType)
   at Newtonsoft.Json.Linq.JToken.ToObject(Type objectType, JsonSerializer jsonSerializer)
   at Newtonsoft.Json.Linq.JToken.ToObject[T](JsonSerializer jsonSerializer)
   at Fable.Remoting.Server.Proxy.makeEndpointProxy@89-17.Invoke(FSharpFunc`2 f, InvocationPropsInt props)
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 464
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 104
  Failed Integration tests for the Diary module HTTP API.Can update review [3 s]

Looks a bit weird, especially since using options in the other direction (server->client) with all sorts of types seems to work just fine.

From paket.lock:

    Fable.Remoting.Client (7.16)
      Fable.Remoting.MsgPack (>= 1.13)
    Fable.Remoting.DotnetClient (3.21)
      Fable.Remoting.Json (>= 2.19)
      Fable.Remoting.MsgPack (>= 1.13)
    Fable.Remoting.Giraffe (5.5)
      Fable.Remoting.Server (>= 5.25) - restriction: || (== net5.0) (&& (== netstandard2.0) (>= net5.0))
    Fable.Remoting.Json (2.19)
    Fable.Remoting.MsgPack (1.13)
    Fable.Remoting.Server (5.25) - restriction: || (== net5.0) (&& (== netstandard2.0) (>= net5.0))
      Fable.Remoting.Json (>= 2.19)
      Fable.Remoting.MsgPack (>= 1.13)
Zaid-Ajaj commented 2 years ago

Hi there @zelenij thanks a lot for filing the issue and providing the relevant information 🙏 I'll have a look

zelenij commented 2 years ago

Interesting observation - the above failure is from tests, running with .NET client. There seems to be no problem when trying the same thing from the browser!

Zaid-Ajaj commented 2 years ago

Hi @zelenij I just tried to reproduce the problem but I wasn't able to. I used a record that has a field of type string option.

Your error message is saying that the error occurs at the following path:

Path '[0].placeId.value'

Which means that the JSON might have had this shape:

{
    "placeId": {
        "value":  "text value here"
     }
}

Can you please provide a sample of code that isn't working?

zelenij commented 2 years ago

I'll try to carve out a stand-alone example.

Did you see my last comment by the way? Might be relevant...

Zaid-Ajaj commented 2 years ago

Did you see my last comment by the way? Might be relevant...

That's what I tested against: using the dotnet client (see this commit)

zelenij commented 2 years ago

Tried the latest version of Fable.Remoting.DotnetClient (3.22) - seems to work now!