Skip to content
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

Support for other WS protocols (ActionCable, Phoenix Channels, etc) #634

Closed
bidluo opened this issue Jul 15, 2019 · 13 comments
Closed

Support for other WS protocols (ActionCable, Phoenix Channels, etc) #634

bidluo opened this issue Jul 15, 2019 · 13 comments
Labels
apollo-websockets enhancement Issues outlining new things we want to do or things that will make our lives as devs easier

Comments

@bidluo
Copy link

bidluo commented Jul 15, 2019

As per Contributing Guidelines, just opening up a forum to discuss the solution before starting work on the change.

Backends like Rails and Phoenix don't easily allow the changing of protocols or a way to rewrite incoming WS messages AFAIK so client side handling seems to be the way to go.
Now that the concept of preflight rewriting is in this project, we could use the same principle with Subscriptions to make the sent/received messages conform to whatever protocol is used.

So in the case of ActionCable (example here) it'd need to wrap the initial subscription call in an AC specific data structure before sending it and conversely unwrap the received data to be processed as normal.

The proposed solution is to expose an optional delegate in the WebSocket transport that will be called before any WebSocket message is written similar to how the new preflight delegate it done.
On the other side it'll also call the delegate when a message is received to handle/unwrap the response before hitting the actual GraphQL logic.

This is unlikely to be a breaking change as the delegate is optional.

@designatednerd
Copy link
Contributor

This seems like an interesting idea, but I gotta admit, I'm not super familiar with these additional protocols. I have a few questions:

  • What are the advantages of using these additional protocols?
  • Are any of these used on top of the Apollo Server stack directly, or are they more generally used across GraphQL clients? (This isn't necessarily a dealbreaker, just trying to figure out how deep a hole we might be digging ourselves here in terms of support)
  • Does the response still wind up the same as other web socket responses? Otherwise I'd be concerned the parsing would break.
  • How would you test this?

@designatednerd designatednerd added apollo-websockets enhancement Issues outlining new things we want to do or things that will make our lives as devs easier labels Jul 15, 2019
@bidluo
Copy link
Author

bidluo commented Jul 15, 2019

  • More necessity than advantage since certain websocket backends only use their own protocol (with no option to customise it) so it makes it difficult to have conformance across different ecosystems. I imagine the reason being is since Rails/Phoenix/etc also deal with frontend they just provide a JS abstraction and leave native implementation up to whoever.
  • Any GraphQL client that communicates with a non JS backend and/or isn’t JS based will have this same issue. Might be talking out my ass here but if the server is a node server then this isn’t even an issue, so realistically the case I’m trying to solve is a bit of an edge case but still something to support.
  • That’s what I hope to achieve, think of this as a middleware in a sense where it’s optionally just serialising/deserialising the messages into something that either can understand. I’ve gotten client -> server to work in my little POC and now working on the reverse.
  • I’ve been thinking about this all day, but frankly since this really is just a delegate with some inout params, shouldn’t be too hard.

I’ll push a PR sometime this week which should hopefully make more sense and clear up any questions. In essence, we’re just exposing a bit more of the websocket network transport flow similar to how the http network transport has its preflight delegate.

@designatednerd
Copy link
Contributor

OK cool - I'll keep an eye out for your PR!

@bidluo
Copy link
Author

bidluo commented Jul 17, 2019

Bit of a hurdle, the current implementation relies on the server sending back an ID which refers to a subscriber on the client side. If the server doesn't, it just errors out, and most non apollo servers don't do this anyway.
I tried implementing this mechanism on the server side but the point of this client change is to reduce friction and the server changes was getting complex as is.

I've been thinking this over for the past few days, looks like there's 2 potential solutions:

  • Delegate method/flag to override ID checks and broadcast to all subscribers after parsing
  • Use multiple WebSocketClients instead of using just one and splitting its data

Any input will be appreciated, I'm learning towards the second option as the first might get a bit dicey with error handling.

@designatednerd
Copy link
Contributor

designatednerd commented Jul 18, 2019

I agree the first option sounds like it could be a serious tangle in terms of error handling.

You're probably still going to need to offer an option to get rid of the ID checks though - I'd say a flag is probably the better option - you'll know when you create the client whether you're going to want to check for IDs or not, and that almost certainly won't change through the lifecycle of the client. If it's known at creation time and never changes, that's a much better case for a flag parameter on the initializer rather than a delegate, IMO

@bidluo
Copy link
Author

bidluo commented Jul 22, 2019

Yeah nah I honestly don't think this is feasible at all, multiple WebSocket clients just straight didn't work and I can't think of any other way to get server -> client working. I have a feeling that it's related to daltoniam/Starscream#551 which has a fix but is yet to be merged

I'm tempted to go down the first route with a flag and let the subscriptions figure out what they need, but that just seems a bit sketch.

As a last resort I'll try this with a WebSocket framework that I'm fairly sure works with multiple connections and see how it goes, but obviously this will expand the scope of this PR quite a lot.
If that fails, just gonna close this off and just write some documentation on this so it's clear as to what's supported and what's not.

@designatednerd
Copy link
Contributor

Yeah any info you can share there would be helpful - the WebSocket stuff was contributed by the community and I'm still honestly wrapping my head around it.

@bidluo
Copy link
Author

bidluo commented Jul 28, 2019

I think this is just a pipe dream tbh, for anyone in the future looking for a solution the best possible alternative is to use silent push to trigger a query/mutation and this is probably preferred from a battery life point too.
Not sure how much I can document since I haven’t even got the existing subscription logic to work lol.

Going to close this and I’d probably recommend referring people here for any subscription related questions that involve anything that strays from the happy path.

@bidluo bidluo closed this as completed Jul 28, 2019
@designatednerd
Copy link
Contributor

OK, thanks very much for sharing!

@Kaakati
Copy link

Kaakati commented Sep 9, 2019

@designatednerd I came across this because I'm looking for a solution for Subscriptions on rails, I've been trying to work around it, but the only feasible solution for me is to use one of the Swift ActionCable libraries with Apollo, I just hope I don't get conflict in Starscream library.

The React version already found their way around it, I hope soon we can get something on rails.

@bidluo
Copy link
Author

bidluo commented Sep 10, 2019

@Kaakati another solution (and the one I went with) was to ditch WebSockets entirely and just use silent push so it's a bit more streamlined. So the server will send a type and an id then the client will make a request using graphql.

It adds a bit of overhead but it just means I don't have to maintain two different systems and deal with manually serialising data since it uses graphql anyway. Also if you have an Android version, the solution can be reused instead of dealing with another round of WS headaches.

@ghost
Copy link

ghost commented Nov 21, 2021

@bidluo Hi, I'm a bit late to this issue, but could you elaborate what you meant by silent push? I'm facing the same issue and trying to figure out a way to handle this. Silent push to the app with data so we can make mutations or queries with the said data seems attractive. Care to explain? Thank you in advance

@designatednerd
Copy link
Contributor

@charlesanim499 The technical term Apple uses is Background Push Notifications - also known as silent push since it sends push notifications without actually showing anything to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apollo-websockets enhancement Issues outlining new things we want to do or things that will make our lives as devs easier
Projects
None yet
Development

No branches or pull requests

3 participants