-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ws] Protocol handling #31
Comments
Do you mind to make a minimal reproduction? 🙏🏼 (this is how i can help to move it forward as soon as possible) |
Since it's more a feature request than a bug report, I don't have a reproduction. But what I would like to do is something of the form: const hooks = defineHooks({
message(peer, message) {
if (peer.websocketProtocol == 'graphql-ws')
handleGraphqlWs(peer, message)
else
handleOldWs(peer, message)
},
upgrade(req) {
if (!req.websocketProtocol in ['graphql-ws', 'subscription-ws'])
return new Response("Upgrade failed due to unsupported protocol", { status: 500 });
},
}) Another idea is perhaps to add all the Edit: Looking at point 4 of the specs it's perhaps better to return the subprotocol directly. I.e. something like upgrade(req) {
if ('graphql-ws' in req.websocketProtocols)
return { protocol: 'graphql-ws' } // prefer graphql-ws if given
else if ('subscription-ws' in req.websocketProtocols)
return { protocol: 'subscription-ws' }
else
return { protocol: null } // This will fail the handshake
}, and crossws then maps this correctly to |
Hi dear @tobiasdiez. With upcoming work of #55 for next release, you can either abort upgrade in Also you should be able to use (closing this issue for triage, but if after release had a chance to test and share your feedback please do 🙏🏼 ) |
That looks awesome! I'll try the new version as soon as it's out. 🚀 |
Describe the feature
I couldn't find a way to get
sec-websocket-protocol
, and based on this value abort the connection or not. Theupgrade
hook looks promising, but it seems only possible to return headers there and not completely abort the upgrade. It would also be nice if one could later get the protocol fromPeer
.(Background: graphql-ws supports only the graphql-ws subprotocol and normally rejects the (deprecated) subscriptions-transport-ws subprotocol. Such a check is currently hard to implement.)
Additional information
The text was updated successfully, but these errors were encountered: