Support reading array of multiple sub protocols from client

[dushaniw]

Story

I am implementing a middle-ware gateway component to manage GraphQL API calls exchanged between client and server. I am trying make my gateway to work with the latest "graphql-ws" protocol, as well as the legacy "subscriptions-transport-ws" protocol (As there will be old users who use it still) I would like to know in default case, if its possible for the server side to accept an array of sub protocols and choose whatever it supports and send that back to client. The MDN explanation for web-sockets states that

client has to ask for a specific subprotocol. To do so, it will send something like this as part of the original handshake:

Sec-WebSocket-Protocol: soap, wamp

or, equivalently:

Sec-WebSocket-Protocol: soap
Sec-WebSocket-Protocol: wamp

Now the server must pick one of the protocols that the client suggested and it supports. If there is more than one, send the first one the client sent. 

My sample server code looks like this (same as your sample server code)

const { WebSocket } = require('ws');
const { buildSchema } = require ('graphql');
const { useServer } = require ('graphql-ws/lib/use/ws');

// Construct a schema, using GraphQL schema language
const schema = buildSchema(`
  type Query {
    hello: String
  }
  type Subscription {
    greetings: String
  }
`);

// The roots provide resolvers for each GraphQL operation
const roots = {
  query: {
    hello: () => 'Hello World!',
  },
  subscription: {
    greetings: async function* sayHiIn5Languages() {
      for (const hi of ['Hi', 'Bonjour', 'Hola', 'Ciao', 'Zdravo']) {
        yield { greetings: hi };
      }
    },
  },
};

const server = new WebSocket.Server({ port: 40000, path : '/graphql', });

  useServer(
    // from the previous step
    { schema, roots },
    server,
  );
  
console.log('Listening to port 4000');

If I write a simple nodejs server using "graphql-ws" and "ws", and if my client send the subprotocol header values as "graphql-transport-ws, graphql-ws" server code always read the first protocol and works fine, but if the header values are in order "graphql-ws, graphql-transport-ws" it doesn't work because the first unsupported protocol is picked.

As I checked, "graphql-ws" also support writing in this in a customizable way (support legacy subscriptions-transport-ws). But, server side developer should explicitly write the protocol reading part for that. Shouldn't choosing values from protocol header array be the default behavior even for simple cases?

Acceptance criteria

Client will be able to send comma separated multiple sub protocol header values in any order.
In simple case, graphql-ws libraray server code will be able to read and pick one of the protocols that the client suggested and it supports (Eg: "graphql-ws").

[enisdenjo]

Hi there,

the client is fixed using "graphql-transport-ws" subprotocol intentionally because it does not know which subprotocol the server chose when multiple are supplied. The client just knows if the connection was accepted or not. This means that sending both subprotocols (["graphql-transport-ws", "graphql-ws"]) from the createClient will break if the latter is selected by the server due to subprotocol mismatch.

Instead, what you should do is check one by one. To check if this library ("graphql-transport-ws") is supported, you can do something like:

function supportsGraphQLTransportWS(url: string): Promise<boolean> {
  return new Promise((resolve) => {
    const client = createClient({
      url, // enpoint you want to check
      retryAttempts: 0, // dont retry, fail immediately
      lazy: false, // connect as soon as the client is created, so as soon as this function is called
      on: {
        closed: () => resolve(false), // connection rejected, NOT supported
        connected: () => {
          resolve(true); // connected = supported
          client.dispose(); // dispose of client after check
        },
      },
    });
  });
}

const supported = await supportsGraphQLTransportWS('wss://somwhere.out/there');
if (supported) {
  // use graphql-ws
} else {
  // use subscriptions-transport-ws
}

Furthermore, regarding the server-side, you can indeed support both the legacy and the current subprotocol. Please refer to the "ws server usage with subscriptions-transport-ws backwards compatibility" recipe.

[enisdenjo]

Added a "Client usage supported check" recipe with c47dcf0.

[leszekhanusz]

Hi,

I'm the maintainer of graphql-python/gql and I really think you should reconsider and allow graphql-ws to work if its subprotocol is present later in the subprotocols provided list by the client.

The client can and should read the answer of the backend in the Sec-WebSocket-Protocol http response header to know which protocol was chosen by the backend.

It should work correctly right now with the current gql code.

If you allow this, it would permit the client to support both the apollo and graphql-ws protocol automatically (currently it works only with your backwards-compatible recipe)

[enisdenjo]

Ah ok, so this is backend focused - you're absolutely right, multiple subprotocols should be supported! PR incoming...

[enisdenjo]

Server now supports and handles multiple subprotocols correctly. @leszekhanusz try it out in version v5.6.1! 😄

[leszekhanusz]

That's great! It works well 🚀 Thank you!

[leszekhanusz]

After further testing, I noticed a very minor issue. The error message printed on the console if the subprotocol is not found is:

WebSocket protocol error occured. It was most likely caused due to an unsupported subprotocol "" requested by the client. graphql-ws implements exclusively the "graphql-transport-ws" subprotocol, please make sure that the client implements it too.

In your code you use $socket.protocol which is empty now:

console.warn(                                                                                                   
  `WebSocket protocol error occured. It was most likely caused due to an ` +                                    
    `unsupported subprotocol "${socket.protocol}" requested by the client. ` +                                  
    `graphql-ws implements exclusively the "${GRAPHQL_TRANSPORT_WS_PROTOCOL}" subprotocol, ` +                  
    'please make sure that the client implements it too.',                                                      
); 

[enisdenjo]

That's expected. As per the specification:

/subprotocol/
Either a single value representing the subprotocol the server
is ready to use or null. The value chosen MUST be derived
from the client's handshake, specifically by selecting one of
the values from the |Sec-WebSocket-Protocol| field that the
server is willing to use for this connection (if any). If the
client's handshake did not contain such a header field or if
the server does not agree to any of the client's requested
subprotocols, the only acceptable value is null. The absence
of such a field is equivalent to the null value (meaning that
if the server does not wish to agree to one of the suggested
subprotocols, it MUST NOT send back a |Sec-WebSocket-Protocol|
header field in its response). The empty string is not the
same as the null value for these purposes and is not a legal
value for this field. The ABNF for the value of this header
field is (token), where the definitions of constructs and
rules are as given in RFC2616.

RFC 6455: 4.2.2. Sending the Server's Opening Handshake

A proper response from the server, when presented with unsupported subprotocol(s) from the client, is to omit the Sec-WebSocket-Protocol or set its value to null, proceed with the upgrade and have the client handle the lack of an agreed subprotocol. It's then the client's responsibility to act accordingly.

For example, ws immediately aborts the handshake with a "Server sent no subprotocol" error. However, if the client still decides to proceed with the upgrade - the socket.protocol on the server will be empty.

I suggest your client does the same, abort the handshake if no subprotocol is agreed upon.