Should incremental execution return an IncrementalStream or a Map containing the InitialResult and a SubsequentResultStream?

[yaacovCR]

Continuing the discussion on the spec PR for the response section and especially this comment:

First, a little history. The initial defer/stream proposals and the published v16 experimental versions returned an ExecutionResult | AsyncIterable<ExecutionResult | ExecutionPatchResult>, i.e. the response was either a map or a stream, where the first item in the stream is the initial execution result, and the remainder of the items are "patches." After plenty of iteration in v17, the equivalent nowadays would be something more like ExecutionResult | AsyncGenerator<InitialIncrementalExecutionResult | SubsequentIncrementalExecutionResult>, and we might further iterate on the naming in graphql/graphql-spec#1124), but the idea is the same, the response should be a regular response or an incremental stream, where the first item differs in type from all of the rest.

Since August 2022, and from version 17.0.0-alpha.1 onwards, however, the graphql-js reference implementation has returned something else, namely:
`ExecutionResult | {
initialResult: InitialIncrementalExecutionResult;
subsequentResults: AsyncGenerator;
} as implemented by @glasser

Advantages of the newer incremental response map type:

1. Typing: explicitly typing the initial result separately from the subsequent results means that the differing types need not be simply asserted/casted. (@glasser claims there that we cannot use the in keyword to test to see whether we have an AsyncGenerator, but I find that to no longer be the case.)
2. Discoverability: always returning a map may help users upgrade, as they will see an object with explicit keys guiding them toward what happened as opposed to iterator internals. (practical concern mentioned by @glasser on that PR).
3. Performance: With the initial approach, a synchronous initial result will be returned asynchronously as the first member of the stream. With the new format, the map with the initialResult/subsequentResults is returned asynchronously only when the first result is asynchronous, and so we do not lose that tick.
4. Flexibility: This allows us to potentially annotate the map with additional fields beyond initialResult or subsequentResults or even extensions that should be scoped to any one result.

Disadvantages:

1. Spec complexity: the additional layer of the map adds a level of indirection.
2. Discrepancy between the result type and the transport format: within the transport world, a multi-part response of any type is in its entirety modelled as a type of stream. It makes sense to speak of the result being a map or a stream, but there is no model of a map containing a stream.
3. Safety: @benjie points out in his response to the comment above that if provided a way to access the initial result before accessing the stream, graphql/spec/library users may forget to close the stream if an error occurs when handling the initial response. This could eventually cause a memory leak, if the subsequentResults stream is not strictly implemented lazily, which it may not be, for example, when earlyExecution: true.

In particular, consider the following code:

if the stream is embedded within a map containing the one might naively write code something like this:

const firstResult = incrementalResponse.initialResult;
doSomethingWithFirstResult(firstResult);
for await (const nextResult of incrementalResponse.subsequentResults) {
  doSomethingWithSubsequentResult(nextResult);
}

if doSomethingWithSubsequentResult throws, it does not call incrementalStream.return() automatically. This could be fixed as follows:

const firstResult = incrementalResponse.initialResult;
try {
  doSomethingWithFirstResult(firstResult);
} catch (e: unknown) {
  incrementalResponse.subsequentResults.return();  // <<<< add this!
  throw(e);
}
for await (const nextResult of incrementalResponse.subsequentResults) {
  doSomethingWithSubsequentResult(nextResult);
}

but if we have the initialResult be the first item in the stream, you could structure your code in the non embedded case as follows:

let processedInitialResult = false;
for await (const result of incrementalStream) {
  if (!processedInitialResult) {
    processedInitialResult = true;
    doSomethingWithFirstResult(result as InitialResult);
    continue;
  }
  doSomethingWithSubsequentResult(nextResult as SubsequentResult);
}

This snippets demonstrates the requirement to use assertions/casts (as InitialResult/as SubsequentResult), a disadvantage of this apparoach, but also lets us get an automatic incrementalStream.return() if anything in the loop throws, based on for await (...) semantics.

Alternatively, because the incrementalStream is typed as a AsyncGenerator which is an AsyncIterableIterator, we could see some users write syntax such as the below to avoid the processedInitialResult check:

const firstResult = await incrementalStream.next() as InitialResult;
doSomethingWithFirstResult(firstResult);
for await (const nextResult of incrementalStream) {
  doSomethingWithSubsequentResult(nextResult as SubsequentResult);
}

which would require them to add the same explicit try { ... } catch(e: unknwon) { incrementalStream.return(); throw(e) } bit, such that forcing the user to access the stream may not be safer after all. @benjie responded (via Discord) that mixing .next() calls and the for await (...) idiom is probably not best practice (one could argue that it's an even an an anti-pattern?). Alternatively, you could argue that forcing the user to access a stream may not help in all cases (i.e. the snippet directly above, but it would help in some, i.e. the snippet with just for await (...) and we should do our best to nudge users into the pit of success.

Finally, one last consideration:

4. Going back to a map means a BREAKING CHANGE to the result format for the next alpha. I personally feel that this would be fine, that's why we have it labelled alpha.

[glasser]

Apollo Server's implementation does have the form you describe as bad, although the function it calls is unlikely to throw.

Re 1, I think the comment I had was more about telling the difference between non-incremental and incremental responses, rather than between telling the difference between initial and subsequent results.

I do think that the typing benefits are nice with the current implementation. Given that initial and subsequent results really are different types, it is helpful for type checking that they are accessed in different ways instead of being differentiated only temporally.

[benjie]

On the "typing" front, who would actually be using these types? To me the incremental delivery format should be consumed pretty much exclusively by GraphQL clients, and users should never see it, only the resulting consistent data. Does that differ to your experience?

[glasser]

Sure, this is for server and client developers, not end users.

[benjie]

Things get even more fun when your subscription operation contains stream/defer, then you end up with nested streams or something similarly unpleasant. In that case, would GraphQL.js return AsyncGenerator<{initialPayload: InitialPayload, subsequentPayloads: AsyncGenerator<IncrementalPayload>}>? I'd rather AsyncGenerator<Payload> I think.

Consider:

let initial = true;
for await (const payload of stream) {
  if (initial) {
    initial = false;
    handleInitialPayload(payload);
  } else {
    handleSubsequentPayload(payload);
  }
  
  // And for subscriptions, reset back to expecting an initial payload for the next event
  if (!payload.hasNext) initial = true;
}

I feel like this gives the strong typing to handleInitialPayload/handleSubsequentPayload without requiring a "stream of map that maybe contains another stream" structure, and without risking failure to process/close the stream.

(It also fits naturally the thought that one event in a subscription completes processing fully before the next event may begin, which in my opinion is the desired state for simplicity and security; nested streams would allow for large deferred/streamed selection sets to stack up in memory on the server as more and more events stream in, potentially leading to OOM and similar issues - very much a DOS vector IMO.)

[robrichard]

This was discussed during the July 2025 WG meeting. The consensus was:

1. We will specify the return type for incremental delivery as a Stream, where the first result is an IniitalExecutionResult, followed by 0 ore more SubsequentExecutionResults, not a map with with initialResult, subsequentResults keys. HTTP and other transports are expected to return the results as part of their byte stream, there are no concepts like promises or async iterables in these transports so a single stream makes most sense in the spec.

2. GraphQL-JS can continue using the current return type. This is an implementation detail and does not violate the spec. Similarly today the spec requires a Map to be returned, but most implementations return a Promise or async task or similar.

[benjie]

where the first result is an IniitalExecutionResult, followed by 0 ore more SubsequentExecutionResults

Just to clarify: you will either return {data, errors} (no stream/defer), or you will return a stream starting with an InitialExecutionResult payload followed by one or more SubsequentExecutionResults - you always need at least a {hasNext: false}.