Subscriptions-transport-ws: Client does not wait for connection_ack on reconnection

Created on 17 Jan 2018  路  8Comments  路  Source: apollographql/subscriptions-transport-ws

When the client reconnects after a disconnect, it appears to flush the unsent message queue before receiving 'connection_ack' from the server. This means that there is no guarantee that your server-side onConnect logic will have completed before the first operation is received. This causes an issue if your onConnect method sets the context, as that context will not be present on the operations that are received.

help wanted

Most helpful comment

At the very least, PROTOCOL.md ought to be updated to say what the expectation is when a client sends a GQL_START during the "connection init phase". (Queue up and execute after GQL_CONNECTION_ACK reply is sent? In order? Before processing any GQL_START that occurs after the connection is initialized? Is it OK for the client to receive GQL_ERROR before GQL_CONNECTION_ACK if the queue is exhausted? etc. etc. - there's a lot of complexity introduced by not simply disallowing subscriptions on an uninitialized connection.)

I understand that the "Messages Flow" section might be non-normative (because it uses the weasel wording of "demonstration [...] in order to get a better understanding"), but even just mentioning that GQL_START might be sent by the client before the "connected phase" would prevent surprises by 3rd parties attempting to implement the protocol on the server side.

All 8 comments

Any updates on this?
@samalexander Have you found any solutions?

Not yet. I think the best solution would be to update the client code to only flush the unsent message queue once it has received a 'connection_ack' message from the server.

I am seeing this happen as well on my client app, and sometimes when I reconnect not all my subscriptions are being re-established.

Might those two things be related?

Are there any updates on this, or is someone able to help point us in the right direction to fix the issue?

Perhaps

https://github.com/apollographql/subscriptions-transport-ws/blob/7f2aa27a02789e572aa678ccc98f915b093a5c2e/src/client.ts#L552

It's in client.connect(); and the comment almost exactly describes the original issue, no?

          // Send CONNECTION_INIT message, no need to wait for connection to success (reduce roundtrips)
          this.sendMessage(undefined, MessageTypes.GQL_CONNECTION_INIT, connectionParams);
          this.flushUnsentMessagesQueue();

Any updates on this?

As a temporary workaround, I created a middleware, that causes any operation to fail until the server returns connection_ack.
That, combined with apollo-link-retry which retries failed operations after a small delay, kind of emulates the expected behaviour:

import { InMemoryCache } from 'apollo-cache-inmemory';
import { RetryLink } from 'apollo-link-retry';
import ApolloClient from 'apollo-client';
import { WebSocketLink } from 'apollo-link-ws';
import { ApolloLink } from 'apollo-link';
import { OperationOptions, SubscriptionClient } from 'subscriptions-transport-ws';

let ack = false;
const subscriptionClient = new SubscriptionClient(GRAPHQL_ENDPOINT, {
    reconnect: true,
    lazy: true,
    connectionParams: () => ({
        authToken: AccountStorage.loadAccount().password
    }),
    connectionCallback: (error, result) => ack = true
});
subscriptionClient.use([
    {
        applyMiddleware(options: OperationOptions, next: Function) {
            if (!ack) {
                throw new Error('not ready');
            }
            next();
        }
    }
]);
subscriptionClient.onDisconnected(() => ack = false);
const webSocketLink = new WebSocketLink(subscriptionClient);

const retryLink = new RetryLink({
    delay: {
        initial: 300,
        max: 60000,
        jitter: true,
    },
    attempts: (count, operation, e) => {
        if (e && e.response && e.response.status === 401) {
            return false;
        }
        return count < 30;
    }
});


const link = ApolloLink.from([
    retryLink,
    webSocketLink,
]);

export const graphqlClient = new ApolloClient({
    link: link,
    cache: new InMemoryCache(),
});

At the very least, PROTOCOL.md ought to be updated to say what the expectation is when a client sends a GQL_START during the "connection init phase". (Queue up and execute after GQL_CONNECTION_ACK reply is sent? In order? Before processing any GQL_START that occurs after the connection is initialized? Is it OK for the client to receive GQL_ERROR before GQL_CONNECTION_ACK if the queue is exhausted? etc. etc. - there's a lot of complexity introduced by not simply disallowing subscriptions on an uninitialized connection.)

I understand that the "Messages Flow" section might be non-normative (because it uses the weasel wording of "demonstration [...] in order to get a better understanding"), but even just mentioning that GQL_START might be sent by the client before the "connected phase" would prevent surprises by 3rd parties attempting to implement the protocol on the server side.

I'm actually wondering if this project is still really active... 馃

Was this page helpful?
0 / 5 - 0 ratings

Related issues

AruXc picture AruXc  路  3Comments

ozeals picture ozeals  路  3Comments

mjasnikovs picture mjasnikovs  路  5Comments

farzd picture farzd  路  3Comments

malditogeek picture malditogeek  路  5Comments