Stream payload index

[robrichard]

Continued from #32

Context

How should a stream payload represent the index of the streamed item? Or should it be represented at all? Is it strictly necessary since the spec enforces payloads are sent in order?(#17).

Three options were discussed at the April 2022 GraphQL WG meeting:

Option A - Stream item index is appended to path array

Example:

{
  "label": "filmsStream",
  "path": ["person", "films", 2],
  "data": [{"title": "Return of the Jedi"}],
  "hasNext": false
}

Advantages

• Consistency: Numbers can appear in any part of the path (nested streams, defer on child of list item)
• User friendly: humans can easily tell the index of the item
• Data correctness: if a payload is missing or out of order, could be used for error checking
• Battle tested? Used by Meta & Relay

Future Expansion

• Out of order payloads
  • Payloads for list items sent out of order or for sparse lists can be supported using payload batching as described in Optimizing incremental delivery #38
• Batch results: How does this evolve to multiple stream items in the same payload?
  • Spec will say that this payload represents an action to "set at a range, where the beginning of the range is at the given index, and the length of the range is the length of the data".
  • Split index into another array? "path": ["person", "films", [2, 3, 4]]

Option B - Stream item index is in a new property

Example:

{
  "label": "filmsStream",
  "path": ["person", "films"],
  "atIndex": 2,
  "data": [{"title": "Return of the Jedi"}],
  "hasNext": false
}

Advantages

• User friendly: humans can easily tell the index of the item
• Data correctness: if a payload is missing or out of order, could be used for error checking

Future Expansion

• Out of order payloads: If we decide to support out of order payloads in the future we will need the index in the payload
• Batch results: How does this evolve to multiple stream items in the same payload?
  • Index indicates the starting position of first item in the batch, subsequent items must be in order?
  • Change property to atIndices? "atIndices": [2, 3, 4]

Option C - Stream item index is not present in the response

Example:

{
  "label": "filmsStream",
  "path": ["person", "films"],
  "data": [{"title": "Return of the Jedi"}],
  "hasNext": false
}

Advantages

• The index is not strictly necessary, since the spec enforces payloads are sent in order (Enforcing delivery order of payloads #17)
  • Although this is a somewhat contrived case, there is the potential for ambiguous payloads as described here.

Future Expansion

• Out of order payloads: Will require changing the payload to add the index somewhere on the response
• Batch results:
  • Items can be appended, assuming they are still required to be streamed in order?
• Are there any other reasons why we might need this data point in the future?

Decision

This was discussed again at the May 2022 WG meeting

The group decided to pursue Option A. After demonstrating the ambiguous case described here, it was determined that the payload must be idempotent. The language in the spec will be updated to indicate that this payload represents "Set at a range, where the beginning of the range is at the given index, and the length of the range is the length of the data."

Consequences

Client libraries must be sure to treat stream payloads as a "Set at index range" operation. Payloads must not be treated as "concat" operations as that can create a bad state in the case of duplicate identical streams as described here.

Status

• defined in spec
• implemented in graphql-js

[yaacovCR]

Great summary! A quick note: Option A could also use atIndices when required just like for B.

[benjie]

[Option C] [Future Expansion] Out of order payloads: Will require changing the payload to add the index somewhere on the response

This will be opted into by a client who requests anyOrder: true (or whatever we call it), so stating that clients that support anyOrder: true must also honour atIndicies seems a reasonable statement.

Further, we're not building the out-of-order stuff now, so we don't really know what we'll need then. Perhaps we'll find that specifying "at ranges" is better: 3-7,9,23-31 - who knows what the future holds? Specifying it now but not implementing any behaviour with it feels wrong and limits future options.

---

My preference is for Option C

• behaviour: append
• pro: concise and straightforward
• pro: no need to validate indexes (since there are none)
• pro: no need to specify/deal with future compexities of out-of-order delivery
• pro: less to explain
• pro: can add atIndicies (or whatever solution we come up with) when necessary, and deal with the complexities at that time (along with the argument that enables it)

Second preference is for Option B with atIndicies:

• behaviour: overwrite at given indicies
• pro: no need to change payload type to allow for out-of-order delivery later
• con: redundant (and especially so if there is a larger linear batch append)
• con: requires at least index validations to assert that a) it starts at the end of the current list, b) the given indicies are monotonically increasing without gaps

I'm not a fan of Option B with atIndex:

• behaviour: overwrite at indexes beginning at atIndex
• con: redundant
• con: to allow for out-of-order batched delivery (e.g. returning items 3, 5 and 7 later) we'd either need 3 separate payloads, or we'd need to add atIndicies: [3, 5, 7] anyway
• con: requires at least index validation (check that atIndex is the same as the existing length of the list)

I dislike Option A:

• behaviour: overwrite at indexes beginning at index in path
• con: same as Option B atIndex (or atIndicies for the array variant)
• additional con: "path" is now misleading, path: ["foo", "bar", 7], data: ["a", "b", "c"] is actually setting ["foo", "bar", 7], ["foo", "bar", 8] and ["foo", "bar", 9]. We're not only modifying ["foo", "bar", 7], we're modifying the others too. Really we're modifying the array at ["foo", "bar"], and specifically the 7,8,9 entries, so the path to me is clearly ["foo", "bar"]. Escaping from the "scope" of ["foo", "bar", 7] feels wrong.

Further note that everything that specifies an index makes one of the following choices:

1. assume and assert that this is an append (not future proof)
2. build this future proof and honour the indexes.

If we're not going to be future proof then we might as well choose option C since adding indexes is adding more work for client and server without much in the way of benefits. If we are going to be future proof (and honour the indexes) then we open a big can of worms:

• what do we do if an index that already has been written to is told to be written to again
• what do we do if the index is too large (e.g. 999999999) and would cause our client to run out of memory?
• how do we handle "gaps" when the stream says it's complete but we've not received everything - especially for clients that are handling the payloads as they come in?
• etc

Should we wish to send down indexes for debugging/help purposes, why not do so with the extensions property when necessary rather than polluting every payload with it?

[yaacovCR]

Example

query {
  user {
    ...DeferredFragment @defer(label: "first")
    ...DeferredFragment @defer(label: "second")
  }
}

fragment DeferredFragment on Query {
  favoriteNumbersInOrder @stream(label: "repeated")
}

could return something like:

{
  data: {
    user: {},
  },
  hasNext: true
}

{
  data: {
    favoriteNumbersInOrder: [],
  },
  path: ["user"],
  label: ["first"],
  hasNext: true
}

{
  data: {
    favoriteNumbersInOrder: [],
  },
  path: ["user"],
  label: ["second"],
  hasNext: true
}

{
  data: [42],
  path: ["user", "favoriteNumbersInOrder"],
  label: ["repeated"],
  hasNext: true
}

{
  data: [1],
  path: ["user", "favoriteNumbersInOrder"],
  label: ["repeated"],
  hasNext: true
}

{
  data: [1],
  path: ["user", "favoriteNumbersInOrder"],
  label: ["repeated"],
  hasNext: true
}

{
  data: [42],
  path: ["user", "favoriteNumbersInOrder"],
  label: ["repeated"],
  hasNext: true
}

{
  hasNext: false
}

[yaacovCR]

above written by hand, i apologize for presumed errors, but I hope idea comes across, but the idea is that the client will not know until the entire stream is over to halve the stream and try to match up the items and get rid of duplicates, and it would seem to be even impossible in many cases for the client to reconstruct the stream in the correct order even given that.

Assuming indices are given, the above becomes easy.

In terms of the can of worms:

what do we do if an index that already has been written to is told to be written to again

write it again or ignore, undefined whether there are conflicts, same as with defer with separate executions, we don't tell client what to do

what do we do if the index is too large (e.g. 999999999) and would cause our client to run out of memory?

client should be prepared for this => a graphql server could return an array with two long of a list as well...

how do we handle "gaps" when the stream says it's complete but we've not received everything - especially for clients that are handling the payloads as they come in?

this is presumably a transport error and should be described in the graphql-over-http and its cousin specs, but not in the graphql spec.

[benjie]

You’re right: index is required to solve duplicate enumeration via independent fragments, good catch.

I’m not sure the latter is a transport error; I’m thinking in terms of a malicious or simply non-compliant server. There’s a lot of things out there that claim to be GraphQL but don’t fully or correctly implement the spec, and GraphQL clients should be tolerant to them where possible.

[yaacovCR]

It could be malicious but I think it's possibly indistinguishable from a transport error and so what to do could be covered by that spec?

[benjie]

If you end with hasNext: false and there's gaps, that's not a transport error.

[robrichard]

This was discussed at the May 2022 WG meeting and has been updated to reflect the decision to pursue "Option A - Stream item index is appended to path array"