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.

Sign up for GitHub

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

Jump to bottom

Netwtonsoft MissingMemberHandling.Error is incompatible with receiving web socket messages #660

Open
JR-Morgan opened this issue Oct 17, 2024 · 3 comments · May be fixed by #661
Open

Netwtonsoft MissingMemberHandling.Error is incompatible with receiving web socket messages #660

JR-Morgan opened this issue Oct 17, 2024 · 3 comments · May be fixed by #661

Comments

@JR-Morgan
Copy link

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.
@rose-a
Copy link
Collaborator

rose-a commented Oct 18, 2024

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.

@JR-Morgan
Copy link
Author

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?

@rose-a
Copy link
Collaborator

rose-a commented Oct 19, 2024

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

@JR-Morgan JR-Morgan linked a pull request Oct 20, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@rose-a @JR-Morgan