Handling loss of websocket connection

[chelliwell]

(Me again lol)
An area I'm concerned about is if a subscription websocket gets disconnected - loss of network, server restart etc.
I will be supplying an intermediate library for people to write their own 'as simple as possible' client app for receiving specific subscription messages from my server, and I would prefer not to burden them with reconnection headaches. - i.e. have either my library or [ideally!] SAHB.GraphQLClient deal with retrying on first subscribe, and with reconnecting if the websocket goes down after that.
I see that OnDisconnected() is empty, so maybe it's 'work in progress' - but it's quite a key area for me which would be great to see enhanced.
I appreciate that it might not be an easy one!

[sahb1239]

The WebSocket class does not have any Connect, however ClientWebSocket does have a Connect method.

Therefore reconnection should be implemented in GraphQLSubscriptionWebSocketClient possible by providing a new event in the IGraphQLSubscriptionClient which could be called on the empty method OnDisconnected(). The two classes GraphQLSubscriptionWebSocketClient and GraphQLSubscriptionClient are seperated because other WebSocket implementations should be supported for example the one I'm using in the test project.

In order to reconnect I also think I should resend all the OperationTypes with the type GQL_START (start) such that the operations will continue to receive data?

So the initial idea of how to support this could be:

• Event Disconnected on IGraphQLSubscriptionClient
• Method Reconnect/ReInitilize (or some other name) on IGraphQLSubscriptionClient
• Configuration of number of retries on GraphQLSubscriptionWebSocketClient
• Handle the Disconnect event in GraphQLSubscriptionWebSocketClient

Do you have any suggrestions to this?

[chelliwell]

I'm afraid you're definitely over-stretching my C# skills there! So I'm afraid I can't really offer anything on how to achieve it - happy to try anything out though and see how it fits with me as a 'user'.
The main needs I see are to (ideally) automatically try to re-establish the socket and subscription, and to have any unhandleable events fed back to the app.

[chelliwell]

The main needs I see are to (ideally) automatically try to re-establish the socket and subscription, and to have any unhandleable events fed back to the app.

And if the re-connect could use a progressive backoff interval.... ;)

[chelliwell]

By the way, you may need/want to watch out for the situation of StartListen in GraphQLSubscriptionClient catching an exception e.g. from the app's DataReceived handler - currently it just calls OnDisconnected.
Should perhaps do this only on catching exception from ConfigureAwait, rather than from OnMessageReceived too? Maybe some finer levels of try/catch in between StartListen and the app's handler - I wouldn't have thought the library needs to care about the latter? ("I simply gave you the data - I'm not going to do change what I'm doing just because you failed to process it successfully")

[sahb1239]

Eventhandlers should generally don't throw exceptions. But you are right the library should handle the exception instead of just calling OnDisconnected.

[chelliwell]

Just coming from the point-of-view of a scruffy library user - quite possibly me lol.

[sahb1239]

Would you create a PR for this change? :)

[chelliwell]

For the possibility of app handler Excepting? I just noticed the possibility whilst trying out what happens currently in such situations. Might be best to try out the strategy for disconnect in general, and I'll feed back on my observations?

[sahb1239]

Yep I think it should not disconnect if the vent handler is throwing an exception.

I will work on the disconnection logic in the next weeks - it would be fine to have some observation on how it should work.

[chelliwell]

I've modified my client lib/app to use this library, and taken a look at various connection situations. My top-level app is able to catch: the server being offline; the server rejecting the websocket connection; and subscription errors such as mismatch between client classes and the server schema.
What I haven't been able to fully hook into is if the server goes offline after the subscription has been made.
My lib has a simple 'subscribe' function that does all the work on your library, and just returns the Task<IGraphQLSubscriptionOperation> to the top-level app (or null/throw if socket/sub errors). My lib hooks the GraphQLSubscriptionClient.Disconnected event and this fires if the server goes down. I'm not sure how this can inform the top-level app of the disconnect though - perhaps I just need to implement a 'disconnect' callback mechanism in my two parts?
Anyway, I'm not sure about how much to attack my stuff if you might be thinking of extra/different 'coping mechanisms' lower down, that's my observations so far.