Skip to content

Support subscriptions #155

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

Closed
smyrick opened this issue Jan 28, 2019 · 8 comments · Fixed by #215
Closed

Support subscriptions #155

smyrick opened this issue Jan 28, 2019 · 8 comments · Fixed by #215
Labels
type: enhancement New feature or request
Milestone

Comments

@smyrick
Copy link
Contributor

smyrick commented Jan 28, 2019

GraphQL Java can support subscriptions: https://www.graphql-java.com/documentation/v11/subscriptions/

We just need to come up with a good way to represent the different type in the schema.

This may be able to be done without a breaking change. Can we just add a @GraphQLSubscription?

@smyrick smyrick added the type: enhancement New feature or request label Jan 28, 2019
@smyrick
Copy link
Contributor Author

smyrick commented Feb 18, 2019

FYI I have a WIP branch here: https://github.com/smyrick/graphql-kotlin/tree/subscribtions

I think we should update the example first to use the webflux as a demo, this can be done without supporting subscriptions but add them after will be much easier and help split up all the example changes I need to do to show how to use a publisher

@smyrick smyrick added this to the 1.0.0 milestone Mar 1, 2019
@shahabio
Copy link

shahabio commented Mar 21, 2019

Whats the status of this? 🙄

@smyrick
Copy link
Contributor Author

smyrick commented Mar 21, 2019

@shahabio I had started working on the update but had trouble getting the example app working with spring websockets. If you want to pick up from my branch you can pull from here: https://github.com/smyrick/graphql-kotlin/tree/subscribtions

@camuthig
Copy link

@smyrick I looked into your branch a bit. At least in my testing, it looks like the issue might be with the headers coming back from the server. I'm getting this error and noticing that the headers are missing on the server response.

Error during WebSocket handshake: Sent non-empty 'Sec-WebSocket-Protocol' header but no response was received

I'm still new to the Spring Boot environment, and largely to Kotlin, so I'm a bit stuck at that point. Looking at the graphql-java spring starter, it looks like they are modifying the handshake specifically to handle this. I'm not sure if there is an easier way to accomplish this or not.

@smyrick
Copy link
Contributor Author

smyrick commented Apr 19, 2019

@camuthig When I run a subscription query through the playground and I put a breakpoint at the top of SubscriptionHandler.handle() I can see the value for session.handshakeInfo.headers["Sec-WebSocket-Protocol"] is graphql-ws so that is still being sent

@camuthig
Copy link

@smyrick I added the same breakpoint and agree that the header is there. However, when I use the dev console on Chromium to look at the response, I don't see it coming back on the response.

The headers using graphql-spring-boot-autoconfigure look something like

Connection: upgrade
Date: Tue, 23 Apr 2019 08:55:26 GMT
Sec-WebSocket-Accept: <token>
Sec-WebSocket-Extensions: permessage-deflate;client_max_window_bits=15
Sec-WebSocket-Protocol: graphql-ws
Upgrade: websocket

But with your branch I'm seeing

connection: upgrade
sec-websocket-accept: <token>
upgrade: websocket

@hinterdupfinger
Copy link

putting

override fun getSubProtocols(): List<String> {
    return listOf("graphql-ws")
}

in SubscriptionHandler(private val graphQL: GraphQL) : WebSocketHandler

should fix the the protocol issue as org.springframework.web.reactive.socket.server.support.HandshakeWebSocketService is checking for supported protocols in the handler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

4 participants