Drop defer/stream support from subscriptions in initial spec proposal?

[robrichard]

Context

Although the spec PR and reference implementation PR currently contain a working implementation of defer and stream in subscription operations, there are still a lot of details that could benefit from further discussion and workshopping (e.g. #54). Given that, I'd like to propose that we remove defer/stream support from subscription operations in the interest of moving the rest of the proposal along.

The complexity of defer/stream in subscriptions arises from the fact that a subscription is a stream of responses and defer and stream also introduce a stream of responses. What is the most appropriate way to combine these streams. The current implementation exhausts all defer/stream payloads from a subscription event before returning the initial payload from the next subscription event. With long-running resolvers and quick firing subscription events, this will result in a backlog of responses being buffered on the server. It should also be noted that this is an existing problem with subscriptions, however defer & stream introduces an opportunity to address this shortcoming.

This was discussed at the Oct 2022 Primary WG where a number of options were raised:

---

Option 1 - Disable via validation rule

Downsides: any subscription query with any defer or stream directive will be rejected. No possibility of re-using fragments between queries/mutations and subscriptions

---

Option 2 - Disable via field error.
This would allow fragments containing defer and stream to be reused inside subscriptions as long as the defer/stream are disabled via a variable on their if argument. Sharing large fragments between query, mutation, and subscription operations is common practice in Relay.

Downsides: errors are raised during execution phase

---

Options ruled out

At the Nov 2022 Primary WG, there was consensus that we should move forward without subscription support, which rules out the following two options.

---

Option 3 - Move forward with defer/stream in subscriptions without multiplexing
Downsides: Multiplexing is arguably better in all cases. By standardizing a non-multiplexed implementation now, clients will have to opt-in to better performance in the future. Ideally multiplexed subscriptions would be the default.

---

Option 4 - Move forward with defer/stream in subscriptions with multiplexing
Downsides: The more likely scenarios of defer/stream being used in queries is held up while the WG figures out the details of specifying multiplexing

---

Option 5 - Server ignores defer/stream directive in subscriptions

This option was ruled out as it would be a breaking change to later add subscription support. If clients always expect that servers will ignore defer and stream, they will not be able to process the response if the server starts supporting subscriptions with multiplexing in a way that the spec describes in the future.

Decision

At the November 2022 (Secondary, EU) WG, there was consensus to move forward with "Option 2 - Disable via field error". This should be accompanied by a new validation rule that prevents @defer or @stream from being used inside a subscription operation without an if argument that is assigned to a variable.

Status

• defined in spec
• implemented in graphql-js (Remove defer/stream support from subscriptions graphql-js#3742)

[maraisr]

motion seconded

[benjie]

Agree with the intent, however we cannot do it via a validation rule in Section 5 (validation) because that phase does not (should not) have access to variables. Instead it must happen after the CoerceVariableValues() step in Section 6 (Execution), which is too late for a validation rule to take place (in my opinion). So we're left with either making the field with @stream / @defer throw, or simply ignoring it. Simply ignoring it makes it uncertain what would happen when we later enable them - is that a breaking change for clients? So I think the field needs to throw (if if: is true)... But that seems non-ideal.

Basically, although I think we should omit @stream/@defer on subscriptions for now, I'm not sure what practical method we would use for doing so.

[robrichard]

It's a similar problem to what we discussed around validating the initialCount argument on @stream, since that could also be provided as a variable (discussion: #24). We decided to raise a field error when a number <0 is passed. So my thoughts were that we would do the same for defer/stream being used on subscriptions. I have a graphql-js PR that implements this: graphql/graphql-js#3742.

[benjie]

The field error approach 👍 I think that's your only viable option TBH.

[robrichard]

This was discussed at the Nov 2022 Primary WG and there was consensus to move forward without subscription support in the initial defer/stream proposal. This rules out options 3 & 4 (now updated in the parent comment).

@leebyron also raised that even when subscription support is fully specced, the spec should allow servers to support defer/stream only in queries and/or mutations. The mechanism we choose here should also be how servers disable defer/stream on their unsupported operation types.

[robrichard]

My favored approach is Option 2 (field error). In my own codebase, I can very easily see us getting into this situation based on how we use subscriptions.

IMO it would be a shame to not be able to take advantage of defer because a fragment is also used in a subscription. It also wouldn't be easy to refactor around this limitation if the defer is used somewhere in a deeply nested fragment. This would be even more painful in the situation that was discussed where a server decides not to support defer/stream on mutations. A compiler step that removes defer in those cases would be helpful, but most clients don't have compiler steps.