graphql / graphql-spec

GraphQL is a query language and execution engine tied to any backend service.
https://spec.graphql.org
14.31k stars 1.13k forks source link

Subscriptions RFC: Bi-directional channels between client and server #278

Closed robzhu closed 7 years ago

robzhu commented 7 years ago

Do we need a "bi-directional" channel between client and server? @taion @stubailo @OlegIlyenko

Continuing the conversation from: https://github.com/facebook/graphql/pull/267#issuecomment-280200236

taion commented 7 years ago

Copying over my response from #267:

The use of SSE v WS is an application-level implementation detail. All that's needed is the server being able to push messages to the client somehow, which distinguishes this from request/response.

The communication isn't really bi-di anyway; it's a request to make a subscription, then a stream from the server for subscription updates. SSE is perfectly fine for this, assuming other technical concerns permit its use.

robzhu commented 7 years ago

I agree, SSE is fine for pushing messages from the server to the client, but SSE alone is not the full story. Perhaps the wording/semantics could be more explicit-- there had to be a request going from client to server for subscribe and unsubscribe. So we essentially have a logical channel where data flows both ways and can be initiated by either client or server. Whether that channel consists of SSE/WS/web hooks can be an implementation detail.

taion commented 7 years ago

You could picture an SSE-only approach where each new subscription is a request to an HTTP endpoint with SSE.

This wouldn't necessarily be a great way to implement things, but there's nothing that would prevent it from working.

stubailo commented 7 years ago

The question here is - do subscriptions require a concept of "unsubscribing"? If so, then that implies that there is some state that tells the server whether or not there is a client to send events to. That's my mental model of what "bidirectional communication" means - the server is aware of whether someone is listening.

taion commented 7 years ago

We're probably getting into hermeneutics a bit, then. From @OlegIlyenko's initial post, there's no reason GraphQL subscriptions can't work over SSE instead of over WebSockets. If that's the only question, then I think we're on the same page here.

OlegIlyenko commented 7 years ago

I think "bi-directional" is a quite broad term. For instance, I consider UDP to be uni-directional. My server can make a broadcast of data based on a subscription query. But as soon as client somehow tells a server the subscription query (which may also be a human communication), the overall communication flow can be considered bi-directional, IMO.

In my opinion, this is a transport concern, so it would not be right to overspecify it in GraphQL spec.

I would suggest to introduce an abstract notion of "infinite result stream" or "result observable" with following properties:

It's just some basic properties of observables as seen in ReactiveX, reactive-streams, etc. I think this can be the right abstraction level that describes a semantics of interaction with GraphQL execution engine but does not imply any particular transport or client-server interaction mechanism.

If we would like to capture the semantics and protocol of subscriptions-transport-ws or similar implementations, I feel it would be better to tackle it as a separate specification (just like relay). I think it's a higher level abstraction and it should be possible to express it on top of the primitives I described above.

What do you think about it?

robzhu commented 7 years ago

@OlegIlyenko, awesome way to break it down.

GraphQL execution engine may experience an unrecoverable error. This will "complete" stream as well.

This is the one point I would object trying to tackle in this issue. I agree with everything else you said. My intent was not to describe specific transport capabilities, merely that the communication flow throughout the subscription lifecycle is bi-directional because there are both client and server initiated communications (in the case of SSE + HTTP, I consider the initial HTTP request to be an instance of client-initiated communication).

I think everyone is mostly in agreement on this topic. The only follow up action is to make the wording more clear. Any objections?

taion commented 7 years ago

None from me; I think the point is that the communication system should in some manner enable the server to push messages to the clients. Whether we call that "bidirectional" is unimportant.

jamesgorman2 commented 7 years ago

I like @OlegIlyenko 's break down, though I think

  • Result stream is infinite, so it may never naturally "complete".
  • Stream may become exhausted. In this case, it is considered "completed" and it will not emit further results
  • GraphQL execution engine may experience an unrecoverable error. This will "complete" stream as well.

is a little unclear. My preference would be for upstream to be able signal no more data so we can close redundant connections if possible (and for downstream to do the same). (It seems like that this the intent here so maybe just a language use issue). Cognate events from Rx are complete, error and unsubscribe which is a nice symmetry.

OlegIlyenko commented 7 years ago

@jamesgorman2 yes, I generally mean the same thing. The completion of a stream is propagated to both the producer and consumer (depending on who initiated it). So that they can do proper cleanup if necessary

robzhu commented 7 years ago

Removed the mention of "bi-directional" with this commit: https://github.com/facebook/graphql/commit/96184f012b87af60128b14afb83946a92109c123