apollographql / apollo-kotlin

:robot:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.7k stars 650 forks source link

Subscriptions over SSE #3756

Open fmatosqg opened 2 years ago

fmatosqg commented 2 years ago

Use case We would like to be able to use GraphQl subscriptions over SSE (Server Sent Events) instead of Websockets.

Out of scope: the merits and demerits of SSE vs Websockets. We just want it and decided it's the best for our project ;)

Describe the solution you'd like

In scope: a new class implementing SubscriptionTransport, for example SseSubscriptionTransport.

In a POC that I'm writing this seems to be sufficient as long as our servers can also implement the protocol that RealSubscriptionManager expects. Otherwise we may need to break down that class in 2 and reimplement some portions of it.

Notes

Our company ( Woolworths ) is investing time in investigating the effort and potential for subscriptions over SSE, and it's already happening internally, ETA for production in the next quarter (Jan-March 2022). A stretch goal would be upstreaming those changes into apollo-kotlin, preferrably v2.5.xxx and subsequent merge into v3.0.x.

At the moment I'm reverse-engineering and reimplementing in SSE client the protocol used between client/server and implemented in RealSubscriptionManager. It's unclear whether our servers would want/be willing to folllow that same protocol, as described in https://github.com/apollographql/subscriptions-transport-ws/blob/master/PROTOCOL.md following link from the PR that first implemented it in apollo-kotlin aka apollo-android (https://github.com/apollographql/apollo-kotlin/issues/754).

As such, it would be really useful to understand the historical origin of such protocol. Is it something born inside Apollo? I see many references to AWS AppSync but can't find the origin of such protocol. Searching in GraphQl foundation specs gave me no luck either.

BoD commented 2 years ago

Hi! 👋 Thanks for reaching out!

Interesting project! A few notes: As you found out, the GraphQL specification itself is agnostic regarding the underlaying protocol to use for subscriptions, which is why other specifications have appeared to supplement it:

As for SSE, I notice this project also exists, which would probably be a good basis for this work? On our side, the existence of a documented protocol is a big plus!

WebSockets / Subscriptions have been an area where we received contributions in the past and other contributions will be welcome! I can say that one big hurdle we have on this area is the difficulty to test / unit test, as servers implementing some of these protocols are not easily available.

One last note: we no longer expect to work on version 2 (except for bug fixes) however now that version 3 has been released, so it is unlikely we would want to add such a feature into version 2.

fmatosqg commented 2 years ago

I've had a good look at enisdenjo's SSE work, and sounds like a great starting point. Keen to adopt that.

I think my plan will be either/both mock server/ktor for dev+test purposes, on top of possibly also running node enisdenjo's for "validating" the protocol implementation.

Regarding v3, I completed migration of my preliminary POC from v2 to v3, and it's easy to notice the improvement in the amount of freedom to write the protocol there. That said, and with the substantial internal api changes, the best course of action seems to be migrate our project to v3 and in parallel develop the client SSE protocol. However our project is quite a big one, and v2->3 migration can get nasty.

fmatosqg commented 2 years ago

Progress report,

Unfortunately my understanding/skills into flexing WsProtocol into something that I need are not great. I'm thinking into making WsProtocol extend a new interface ( Protocol ) and then making myself a new (abstract?) class that extends that, like SseProtocol. Because for example I have no use for a webSocketConnection --> class MyProtocol :WsProtocol(webSocketConnection = UnusedWebSocketConnection()...)

Maybe I'll also need to bend a few ApolloClient builders but so far I think they serve me right.

I recently joined graphql discord, my timezone is +10h so not sure if we can cross paths over there to have a chat. Is there any idea on how we can have a quick convo on text/voice? Engineers, not managers at this moment pls.

BoD commented 2 years ago

Hi! Thanks for the update.

What you say about having a Protocol interface makes sense, basically extracting / making it independent of the underlying "pipe". On the other hand, could the WebSocketConnection interface still be used for SSE (if we ignore the naming for now)? If yes it may mean WsProtocol could still be used as-is?

We'd be happy to talk more, and the best way is on the Kotlin Slack in the #apollo-kotlin channel (get an invite from here if you're not already there).

fmatosqg commented 2 years ago

For better understanding where I am, I'm now confident I can deliver the feature the way I want without code changes to Apollo-kotlin v3. So my goal is making SSE work in my company, and my stretch goal is making high-quality code changes to apollo-kotlin in order for it to officially support.

context

WebSocketConnection won't do (at least for now), since I need to handle 2 connections: a SSE + side channel. It's important to clarify here that I'm pursuiong the HTTP1 where all subscriptions travel through the same SSE channel (instead of the HTTP2 approach where each subscription has its own long-lived SSE channel).

Side channel's purpose is to add and remove subscriptions, as well as providing the initial handshake. That's where the subscription request travels.

Long lived channel (aka SSE channel) purpose is to get subscription responses from server to client.

problem

Having an interface like WebSocketConnection is ok-ish. But handling 2 connections through a single object: WebSocketConnection is very hard at best because of suspend fun receive(): String being used to get data from 2 different types of channel. If it were something like suspend fun receive(): WebSocketResponse then it would be easier to know if we are getting responses from side channel or sse channel.

It can be done but it gets quite ugly, so I'll look into alternatives, like completely not using WebSocketConnection and instead handling 2 objects, one for each type of channel.

temp solution


open class SseProtocol(
   channelFactory: Channel.ChannelFactory,
) : WsProtocol(UnusedWebSocketConnection(), UnusedListener()) {

So far, and I don't have plans on changing this, UnusedWebSocketConnection and UnusedListener are full of TODOs and only purpose is to let me inherit WsProtocol.

Channel.ChannelFactory is a bit like WebSocketEngine, except it has 2 methods so I can make either Rest GET (side channel) or SSE (long-lived channel).

Potentially I can have 2 objects of WebSocketEngine so I handle 2 types of WebSocketConnection, one for GET another for SSE. But neither of them will be truly web sockets, so I'm not sure we want this.

acao commented 2 years ago

Hey so, if you're looking for a quick graphql server that supports SSE, this one is great!

https://github.com/contra/graphql-helix

kansson commented 1 year ago

Hi there! Are there any plans to include this in the v4 release? Here is a working multiplatform example that reads server-sent evets using Ktor https://github.com/JurajBegovac/ktor-kmm-sse. Using Ktor for this makes sense and maybe #3796 could be done at the same time?

BoD commented 1 year ago

@hanssonduck This is currently not being worked on by the team and not planned to be part of v4, but we would certainly welcome a contribution for it. Note: as for #3796, having this in a dedicated module would probably be the best, if possible.

kansson commented 1 year ago

@BoD okay! Might work on this in the coming week if I have time. Should the new subscription transport also be its own module or is it fine to add the core Ktor dependency along side okio in the common code?

BoD commented 1 year ago

@hanssonduck it would definitely be a plus if this is separated in its own module