graphql-client icon indicating copy to clipboard operation
graphql-client copied to clipboard

Netwtonsoft `MissingMemberHandling.Error` is incompatible with receiving web socket messages

Open JR-Morgan opened this issue 1 year ago • 3 comments
trafficstars

My goal is to be able to utilise the MissingMemberHandling.Error option of the JsonSerializerSettings, instead of the default MissingMemberHandling.Ignore.

Why? because its a very useful way to ensure that the classes I'm crafting as deserialization targets for my queries actually match the queries being sent. It makes it impossible to accidently query for something that will be ignored during serialization, as this will throw a JsonSerializationException

This works perfectly for regular queries and mutations. But not for subscriptions or UseWebSocketForQueriesAndMutations = true

Instead, I get this exception thrown deep from ReceiveWebsocketMessagesAsync

Newtonsoft.Json.JsonSerializationException: Could not find member 'payload' on object of type 'WebsocketMessageWrapper'. Path 'payload', line 1, position 65.
   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.JsonSerializer.Deserialize[T](JsonReader reader)
   at GraphQL.Client.Serializer.Newtonsoft.NewtonsoftJsonSerializer.DeserializeFromUtf8Stream[T](Stream stream) in /_/src/GraphQL.Client.Serializer.Newtonsoft/NewtonsoftJsonSerializer.cs:line 54
   at GraphQL.Client.Serializer.Newtonsoft.NewtonsoftJsonSerializer.DeserializeToWebsocketResponseWrapperAsync(Stream stream) in /_/src/GraphQL.Client.Serializer.Newtonsoft/NewtonsoftJsonSerializer.cs:line 41
   at GraphQL.Client.Http.Websocket.GraphQLHttpWebSocket.ReceiveWebsocketMessagesAsync() in /_/src/GraphQL.Client/Websocket/GraphQLHttpWebSocket.cs:line 524

I've captured the offending response from the server. And it does indeed have a payload property, (the value of which is all from my GraphQL api)

  "{\"type\":\"data\",\"id\":\"80fdadc12c0b44bc84115ce7231cb096\",\"payload\":{\"data\":{\"data\":{\"comment\":{\"id\":\"1408af422d\"},\"id\":\"1408af422d\",\"type\":\"CREATED\"}}}}"

But poking around a little, this seems like an indented outcome seeing as WebsocketMessageWrapper does not need to store the payload


I was wondering if this was intentional, as it's preventing me from using this serializer setting for my entire SDK.

JR-Morgan avatar Oct 17 '24 21:10 JR-Morgan

Hi, the problem is that the payload field is not always there in the websocket protocol. What's in there can only be decided after the first deserialization and evaluating the type field.

The easiest way to solve your problem you would be to implement your own NewtonsoftJsonSerializer, which uses a different instance of json serializer settings for DeserializeToWebsocketResponseWrapperAsync, which is where your exception is thrown and which is the "partial" serialization step needed to get the type field from the message.

I'd be glad to incorporate that "separation" of serializer settings into the library if you submit that as a PR.

rose-a avatar Oct 18 '24 06:10 rose-a

Amazing, thanks. I'm already using a fork of the NewtonsoftJsonSerializer so tweaking it is quite easy for me.

https://github.com/specklesystems/speckle-sharp-sdk/blob/b986a1bc6f737e1a83b861e0c9c1ffab361ec29a/src/Speckle.Sdk/Api/GraphQL/Serializer/NewtonsoftJsonSerializer.cs#L49

I've just tested with making the DeserializeToWebsocketResponseWrapperAsync always use the DefaultJsonSerializerSettings , as this was simplest, and I don't see any valid reason users would ever want to customise this JsonSerializerSetting since its only used for deserializing WebsocketMessageWrappers.

This is now working great for me 😁

Would you like this as a PR?

JR-Morgan avatar Oct 18 '24 10:10 JR-Morgan

Yea please do a PR. I think this should have been there from the start...

rose-a avatar Oct 19 '24 20:10 rose-a