-
Notifications
You must be signed in to change notification settings - Fork 425
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
Add juniper_graphql_transport_ws crate for new subscription protocol #1158
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file started out as a copy/paste from juniper_graphql_ws
. The control flow is mostly identical except the new protocol defines several situations in which the server must close the connection with certain codes. So instead of a stream of messages, the connection is a stream of Output::Message(msg)
/ Output::Close{code, message}
.
CI failures are unrelated: #1159 |
2eae1c5
to
a6ae386
Compare
@ccbrown first of all, thank you for spending time on this Since the control flow doesn't differ much, maybe it's better to implement this in the existing |
A common use-case is for servers to support both simultaneously and negotiate the correct protocol to use on each connection, as discussed in seanmonstar/warp#760. To support that use-case, we can't force the users into one or the other at compile-time. I also think the changes, while relatively minor, are pervasive enough that it makes switching between the protocols via conditionals very messy and hard to follow. |
Dropping a message here to say that we have been using this for the past months in our systems. All queries, mutations and subscriptions seem to be working great, with clients communication to our server using JS & Rust implementations. Thank you very much for this work, saved us :) |
I'm going to merge this as-is, thank you! |
@ccbrown agree. Thanks! |
This adds a new crate:
juniper_graphql_transport_ws
This crate implements the new graphql-transport-ws protocol as defined here.
This protocol obsoletes the old graphql-ws protocol. Unfortunately the naming situation is a bit confusing as the old protocol uses the "graphql-ws" WebSocket subprotocol and the new protocol's reference implementation is in a repo named "graphql-ws". But the right way to go to me seems to be to map the WebSocket subprotocols into the crate names.
See #1022 for discussion.
This also adds a
serve_graphql_transport_ws
function to thejuniper_warp
crate, which is analogous toserve_graphql_ws
. Ideally we would do some auto-negotiation and use the correct protocol intelligently, but it's a little unclear where this should go or what the cleanest way to implement it would be. It is possible to do this in Warp, but Warp doesn't provide first-class support for it. You can read about this exact use-case in seanmonstar/warp#760. I'm punting on that part for a follow-up PR maybe. Same with otherjuniper_actix
changes.Closes #1022.