I visited this PR after looking up an issue with an earlier version of this library (v0.6.4). When an error is raised and it ends up in this branch, the connection is not reset, so any retry attempt to use subscriptionClient.Run is not effective. In that case, we would need to explicitly Reset() the subscriptionClient, so it can be reused.

In this PR, I can see that Close() is called when this point is reached, so the subscriptionClient cannot be reused, by consuming the error from Run() and recalling Run().
Is this change intentional, cause this changes the behaviour of the subscriptionClient quite significantly IMO?

UPDATE: the above statement was incorrect, so it seems that the subscriptionClient can indeed be reused multiple times, by calling Run -> Close -> Run -> Close, is this correct?

The onError line is explained in the document https://github.com/hasura/go-graphql-client#handle-connection-error. It's intentional. The subscription client tries to retry subscriptions if the GraphQL server is outage (disconnected). For user-defined errors, we should let the user decide whether to end the subscription.

The Close method clears all subscriptions and closes the WebSocket connection. In theory you can continue to Run the subscription client after being Close. However, you need to resubscribe subscriptions. It's better to use Reset function which resets subscription states and runs a new WebSocket connection

Hi, thanks for your answer!

So, in case a user-defined error is raised how can the user perform a retry? It seems the OnError should consume the error and return nil, so the client is not closed. Is it suggested to call the Reset in the OnError function? Sorry, its not crystal-clear to me from the documentation, although this might be the reason for providing the subscriptionClient instance in the callback.

Also, what if the user of the library applies its own retry-mechanism to the client, and still tries to reuse the client?

It seems to me its now impossible to properly decide if the retry should call Run or Reset because if the client failed in the initialisation phase, then Run should be executed, while Reset is only useful after the client has initialised at least once. This information is held by isRunning which is internal, so the user cannot make a proper decision if Run or Reset should be called.

It seems to me that the current version of Reset can replace Run because it is actually a superset (not an intersection) of what Run does, so it can be called multiple times (idempotent) while the state of the subscription client defines what actions will be triggered. This could also simplify the the public interface of the subscription client by removing Reset in favour of Run. What do you think?

That makes sense. Anyway, the client should handle the retry mechanism when there is some connection problem only. For other errors, the client should stop and let developers rerun subscriptions themselves. We shouldn't force retrying for every error because it will lead to an endless loop.

The change is updated here #76. Feel free to leave some feedback. Thanks!

Thanks, I am glad to provide review/feedback!