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

subscriptionStream.Subscribe response never occurs (fails silently) if deserialization fails

Open BillBaird opened this issue 4 years ago • 2 comments
trafficstars

I have a case where I receive subscription messages that among other things contain enum values. Everything is fine if deserialization works, but if it fails (in this case because of #311), neither the exceptionHandler provided to CreateSubscriptionStream nor the IObservable<T> provided to the Subscribe method is ever called. The subscription message is effectively dropped.

I discovered this when investigating why some messages got through and others did not. It turned out that if the enum values all where "Regular" values (and not PascalCase or camelCase - see #311) everything was fine. But for PascalCase, the message failed. Digging deeper, I discovered that the NewtonSoft deserializer does raise an exception informing of the error Newtonsoft.Json.JsonSerializationException: Error converting value "PASCAL_CASE" to type 'TestEnum'. Path ..., but that error causes the message to never get to either the exceptionHandler or IObservable<T> callback. I would expect one of the two to be called. Of course if it were the IObservable<GraphQLResponse<MyType>>, Errors should contain error information.

This issue is about making sure an error is raised when deserialization throws an exception in case any other exceptions, such as the one discussed in #311 also crop up.

BillBaird avatar Dec 11 '20 21:12 BillBaird

My subscription does also fail silently. Can you elaborate how you got the serializer to throw that message?

maaft avatar Dec 14 '20 12:12 maaft

@maaft, In my case, I knew it was the deserialization of the enum from past diagnosis. I was then able to inject my own enum converter (as described at the end of #310) and place a try-catch around the call to the ConstantCaseEnumConverter (StringEnumConverter) which it derived from. (I placed the try-catch around the call to base.ReadJson). As expected, the exception was caught and I was able to examine it and understand why deserialization was failing. Here is my full converter.

public class ConstantCaseEnumWorkaroundConverter : ConstantCaseEnumConverter
{
    public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
    {
        if (reader.TokenType == JsonToken.String)
        {
            string? enumText = reader.Value?.ToString();
            enumText = enumText.Replace("_", "");
            if (Enum.TryParse(objectType, enumText, true, out var enumVal))
                return enumVal;
            return base.ReadJson(reader, objectType, existingValue, serializer);
        }
        return base.ReadJson(reader, objectType, existingValue, serializer);
    }
}

Then, when you create your GraphQLHttpClient you will need to tell it to use your converter:

// Create JsonSerializer
var serializerSettings = new JsonSerializerSettings
{
    ContractResolver = new CamelCasePropertyNamesContractResolver {IgnoreIsSpecifiedMembers = true},
    MissingMemberHandling = MissingMemberHandling.Ignore
};
serializerSettings.Converters.Add(new ConstantCaseEnumWorkaroundConverter());

// See https://github.com/graphql-dotnet/graphql-client/issues/311
var jsonSerializer = new NewtonsoftJsonSerializer(serializerSettings);

// Establish Client
_subscriptionClient = new GraphQLHttpClient(o =>
    {
        o.WebSocketEndPoint = new Uri("https://yourUri");
        o.UseWebSocketForQueriesAndMutations = false;
        o.OnWebsocketConnected = OnWebSocketConnectAsync;
    },
    jsonSerializer);

Assuming your silent failure issue is in deserialization, this approach should work, although if it is not the enum issue, you would need to make a custom converter for your outer type and use that instead. This example uses the NewtonSoft converters, but SystemTextJson converters should work similarly.

Ultimately, I think the solution to this silent deserialization issue will be with adding or modifying a try-catch in GraphQL.Client, but I have yet to search for that. The problem is potentially bigger than just deserialization as well. I also haven't experimented with whether this affects query and mutation results.

BillBaird avatar Dec 14 '20 15:12 BillBaird