enisdenjo / graphql-ws

Coherent, zero-dependency, lazy, simple, GraphQL over WebSocket Protocol compliant server and client.
https://the-guild.dev/graphql/ws
MIT License
1.75k stars 162 forks source link

WebSocket Connection Fails Due to Empty Protocol in Deno.upgradeWebSocket with Deno 2 #587

Closed wmadfaa closed 3 weeks ago

wmadfaa commented 1 month ago

Screenshot N/A

Expected Behaviour Using graphql-ws@^5.16.0 with Deno 2.0.3, the WebSocket connection should successfully establish with the GRAPHQL_TRANSPORT_WS_PROTOCOL protocol in accordance with the sample code provided by the library.

Actual Behaviour The WebSocket connection fails because Deno.upgradeWebSocket returns a WebSocket object with an empty protocol property, causing a protocol mismatch and connection cancellation in the makeServer handler.

Debug Information This issue appears to stem from a conditional check within the makeServer function, where an empty protocol string results in an unsuccessful connection:

Further Information The issue is likely rooted in Deno's handling of the protocol property in Deno.upgradeWebSocket. For now, the workaround is to use a Proxy object to intercept and set the protocol as GRAPHQL_TRANSPORT_WS_PROTOCOL, as shown below:


import { makeHandler, GRAPHQL_TRANSPORT_WS_PROTOCOL } from 'https://esm.sh/graphql-ws/lib/use/deno';
import { schema } from './previous-step.ts';

const handler = makeHandler({ schema });

Deno.serve({ port: 4000 }, (req: Request) => {
  const [path, _search] = req.url.split('?');
  if (!path.endsWith('/graphql')) {
    return new Response('Not Found', { status: 404 });
  }
  if (req.headers.get('upgrade') != 'websocket') {
    return new Response('Upgrade Required', { status: 426 });
  }
  const { socket, response } = Deno.upgradeWebSocket(req, {
    protocol: GRAPHQL_TRANSPORT_WS_PROTOCOL,
    idleTimeout: 12_000,
  });

  const socketProxy = new Proxy(socket, {
    get: (target, prop, receiver) => {
      if (prop === "protocol" && target.protocol === "") {
        return GRAPHQL_TRANSPORT_WS_PROTOCOL;
      }
      return Reflect.get(target, prop, receiver);
    },
  });

  handler(socketProxy);
  return response;
});

The above workaround can help other developers temporarily address the issue until a fix is implemented in Deno.
enisdenjo commented 3 weeks ago

Hey there! Thanks for reproting this and good catch on how to fix it in the meantime. Do you think it's worth implementing in the library or adding a comment in the documentation for anyone that may face the same issue?

wmadfaa commented 3 weeks ago

Hey there! Thanks for reproting this and good catch on how to fix it in the meantime. Do you think it's worth implementing in the library or adding a comment in the documentation for anyone that may face the same issue?

Hi!, yes adding a comment in the documentation is a good idea! I'll close the issue and open a pr with the changes asap