Connection closed after onError() in the subscription

[Lana11s]

Hello,

I'm using graphql-ws with Apollo Client and lately updated from 4.2.1 to 4.2.2.

It seems that if there is an onError event in the subscription, a complete event is submitted and the subscription gets closed:

My question is whether this behavior is intended/expected?

In the 4.2.1, only error() was invoked, without complete()

[enisdenjo]

Hey there, yes, this is intended behaviour. Following the Observer pattern both the error or complete stop the observable/sink. One of them needs to be invoked to complete the execution (one for erroneous one for successful), but never both.

[enisdenjo]

Furthermore, the Complete message should not be sent from the client if it receives an Error message. Hence the completed = true:

graphql-ws/src/client.ts

Lines 552 to 555 in cd70a49

	case MessageType.Error: {
	completed = true;
	sink.error(message.payload);
	releaser();

The releaser checks the completed flag, and if true - it doesnt dispatch the Complete message:

graphql-ws/src/client.ts

Lines 578 to 585 in cd70a49

	if (!completed && socket.readyState === WebSocketImpl.OPEN)
	// if not completed already and socket is open, send complete message to server on release
	socket.send(
	stringifyMessage<MessageType.Complete>({
	id,
	type: MessageType.Complete,
	}),
	);

[Lana11s]

Hey @enisdenjo,
thanks for the quick response and clarification!

I am trying to resubscribe when error() callback is triggered.
But now it results in the invocation of complete() of the new subscription.

Best regards

[enisdenjo]

Urm, I dont quite understand. Could you elaborate your issue a bit more? New subscriptions have nothing to do with old/completed/errored subscriptions. Note that resubscribing is not done automatically, the user/you have to resubscribe by calling client.subscribe again.

[Lana11s]

Simplified, there is following:

 // 1) initial call
function subscribe() {
    client.subscribe({ query: QUERY }).subscribe(createObserver()) // Apollo Client with WebSocketLink as in readme-examle.
}

function createObserver<T>(): ZenObservable.Observer<T> {
    return {
        error(error: any) {
            subscribe() // 2) onError: resubscribe
        },
        complete() {
            // 3) gets called for the newly created/subscribed observer
        }
    }
}

[enisdenjo]

It is possible that you're getting the complete from the first subscription. Can you confirm that it is not?

Also, can you share your WebSocket messages from the browser DevTools?

A repro would be the best and easiest way for me to check the behaviour.

[enisdenjo]

Note that, with the current version of the lib, both error and complete do get called. There is a TODO here:

graphql-ws/src/client.ts

Lines 552 to 559 in cd70a49

	case MessageType.Error: {
	completed = true;
	sink.error(message.payload);
	releaser();
	// TODO-db-201025 calling releaser will complete the sink, meaning that both the `error` and `complete` will be
	// called. neither promises or observables care; once they settle, additional calls to the resolvers will be ignored
	return;
	}

So, it is probably the first subscription calling the complete.

[Lana11s]

yes, complete comes from the second subscription. If I modify the observer, then I get:

function createObserver<T>(): ZenObservable.Observer<T> {
    return {
        error(error: any) {
            client.subscribe({ query: QUERY }).subscribe(createSecondObserver()) // 2) onError: resubscribe
        },
        complete() {
            // not called
        }
    }
}
function createSecondObserver<T>(): ZenObservable.Observer<T> {
    return {
        complete() {
            // 3) gets called for the newly created/subscribed observer
        }
    }
}

It is running within a test, where at first an error-event sent from server followed by next-event afterwards.

If a closed listener is registered on the WebSocketLink, it is notified right after resubscription:

wsLink.client.on("closed", (event: any) => {
    // invoked only if resubscription created
})

[enisdenjo]

Hmm, weird. You might be onto something. But, sadly, I cannot put a dot on what...

I'd be very appreciative if you could provide me with a reproduction that I can dig around. I dont know where to start really... It can be a factor of things, from the server, through the client, all the way to Apollo stack.

[Lana11s]

Hello @enisdenjo,

I found out that there is some timing issue (perhaps only with my test), so that with

error(error: any) {
    new Promise<void>((resolve, _) => {
                setTimeout(() => {
                    resolve()
                }, 300)
            }).then((_) => {
            client.subscribe({ query: QUERY }).subscribe(createSecondObserver()) // 2) onError: resubscribe
    })
}

it works fine. I'm currently on it.

This code currently not in a github repository yet, I will try to make an example project for it and let you know.

Thank you for your support!

[enisdenjo]

Ok, cool! Please keep me up-to-date, I'd like to fix this if coming from graphql-ws. 😄

[enisdenjo]

🎉 This issue has been resolved in version 4.4.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[enisdenjo]

Hey @Lana11s, good news - I think #173 was experiencing something similar. I pushed a fix and feel that this issue should be resolved with it too.

Can you please try v4.4.3 out with your original problem?

[Lana11s]

Hey @enisdenjo
thank you for the fix!
But I'm still not sure if this resolves my problem completely.

I was able to achieve the desired behavior by setting the 'keepAlive' property. Using it, the socket remains open and the re-subscription can be initialized.

I have also some other question:
Is it intended that 'locks' can have negative values?
I added some println and got:

    console.log
      locks: 1

      at node_modules/graphql-ws/lib/client.js:285:25

    console.log
      locks: 2

      at node_modules/graphql-ws/lib/client.js:285:25

    console.log
      locks: 1

      at releaser (node_modules/graphql-ws/lib/client.js:321:37)

    console.log
      locks: 0

      at releaser (node_modules/graphql-ws/lib/client.js:321:37)

    console.log
      locks: 0

      at node_modules/graphql-ws/lib/client.js:193:29

    console.log
      locks: -1

      at releaser (node_modules/graphql-ws/lib/client.js:321:37)

    console.log
      locks: -1

      at node_modules/graphql-ws/lib/client.js:193:29

[enisdenjo]

Great find! Seems like it is indeed possible for the locks to go negative if you dispose of a subscription multiple times. This behaviour is potentially causing your misbehaviour, this is how:

1. Create 2 subscriptions
2. Dispose/complete the 2nd subscription 2 times
3. Locks decrement from 2 to 0
4. Since lazy = true and locks = 0, the connection gracefully closes - invoking the complete on the 2nd subscription

[enisdenjo]

🎉 This issue has been resolved in version 4.4.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[enisdenjo]

Hey hey @Lana11s, I made sure subscriptions can be disposed only once (locks shouldnt go negative) in v4.4.4.

Could you please try again? 😄

[Lana11s]

Hey @enisdenjo,

I tested it and it works as expected, even without my workaround with 'keepAlive'.
After first subsription is terminated with an onError() event, the second one can be registered successfully, without getting closed :)
And the locks-counter remains positive ;)

Thank you!

[enisdenjo]

Wohoo, nice! Glad it works now without any workarounds in place. Thank you for sticking around and providing valuable insights. 😄