[RFC] Directive proposal for opting out of null bubbling

[benjie]

This PR builds on #1065.

This introduces a directive on operations that disables the null/error propagation behavior by treating all Non-Null types as if they were Semantic-Non-Null types (see #1065).

The specific name of this directive (currently @disableNullPropagation) is open to workshopping:

• @noBubblesPlz 😉 or @tepid 🤣
• @disableNullPropagation / @disableErrorPropagation / @noNullPropagation / @noPropagation / @nullOnError / etc
• @localErrors
• @dontHandleErrorsForMeIKnowWhatImDoing
• {your suggestion here}

Implemented in graphql/graphql-js#4192 as part of semantic non-null support.

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	e58ab2b
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/651e9d35c809180008a5458c
😎 Deploy Preview	https://deploy-preview-1050--graphql-spec-draft.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[bbakerman]

in terms of naming - we have a proposed PR on graphql-java

graphql-java/graphql-java#3772

@errorHandling(onError : NULL) or @errorHandling(onError : PROPOGATE)

This is not merged (yet) but its a name suggestion I would like to link here

[benjie]

Another option:

enum __ErrorBehavior {
  """
  Non-nullable positions that error cause the error to propagate to the nearest nullable
  ancestor position. The error is added to the "errors" list.
  """
  PROPAGATE
  
  """
  Positions that error are replaced with a `null` and an error is added to the "errors"
  list.
  """
  NULL
  
  """
  If any error occurs, abort the entire request and just return the error in the "errors"
  list. (No partial success.)
  """
  ABORT
  
  """
  Positions that error are replaced with an inline representation of the error, and
  the path to the error is added to the "errorPaths" list.
  """
  INLINE
}

directive @behavior(onError: __ErrorBehavior! = PROPAGATE) on QUERY | MUTATION | SUBSCRIPTION

(To be clear: I'm not proposing to implement ABORT or INLINE - these are just examples to justify why we might consider using an enum rather than a boolean.)

Rather than going error-centric for this, I felt like a single directive to handle many behaviors might make sense. We could later expand this so that you can do things like @behavior(onDeprecated: ERROR, onError: ABORT).

[martinbonnin]

Agree 100% with one nitpick: I would merge INLINE and NULL.

NULL being somewhat of a consequence of the JSON encoding but if we want to support other encodings later on, a more general wording might help. Maybe something like:

enum __ErrorBehavior {
  """
  Positions that error are replaced with a `null` and an error is added to the "errors"
  list. Non-JSON encodings may decide to represent errors inline.
  """
  LOCAL

  """
  Non-nullable positions that error cause the error to propagate to the nearest nullable
  ancestor position. The error is added to the "errors" list.
  """
  PROPAGATE
  
  """
  If any error occurs, abort the entire request and just return the error in the "errors"
  list. (No partial success.)
  """
  ABORT
}

[yaacovCR]

Non-JSON encodings may decide to represent errors inline.

I think with @benjie's version, inlining errors could work even with JSON encodings. It's often said that we cannot embed errors within the response because a custom scalar could be anything, but we can use an errorPaths list to actually distinguish between the two, as long as the client would be aware of this possibility, which could be assumed if they opt into it with the INLINE option.

In theory, whether errors are inlined is orthogonal to whether errors propagate or cause execution to abort, and could be a separate argument. From that perspective, propagation vs abortion vs neither could be an errorBlastRadius argument with an Enum of these options while inlining behavior could be represented by a separate inlineErrors argument of type Boolean. I think the counterargument to this theoretical orthogonality is that if you are inlining errors, then you must be using an error-aware client, and then you would never need to propagate. Aborting on the first error would definitely make inlining redundant.

A counter-counter-argument could be that we have assumed that the only reason to propagate errors is for null-protection, i.e. type safety reasons on the client. I think that is the general assumption. But there is another potential advantage. Propagation allows us to skip executing sibling fields that are presumably unnecessary in the context of a catastrophic null within a non-nullable field, reducing the overall execution time and server load. I am not sure how much I believe that is a practical concern (and in our reference implementation it reduces execution time, but not server load, as we do not yet cancel the async work). but under that logic, one could see a scenario for @behavior(errorBlastRadius: PROPAGATE, inlineErrors: true).

I might still argue to separate them, kind of makes things more descriptive.

[martinbonnin]

It's often said that we cannot embed errors within the response because a custom scalar could be anything, but we can use an errorPaths list to actually distinguish between the two

@yaacovCR Right. Apologies I misread the description of INLINE above. I was really expecting something called "inline" to unlock response streaming (reduce buffering on the server and just send bytes as they are returned from the resolvers), which errorPath doesn't allow because it needs to be transmitted first for the client to make informed decisions.

All in all, I would probably challenge the naming there but this is probably a question for later, if we decide to implement it. The good thing about the enum solution is that we can start with just PROPAGATE and NULL (or maybe DO_NOT_PROPAGATE?) and if we find the need for ABORT or INLINE we can always add them later (therefore preserving option value 🙌 ).

[benjie]

So I thought about this more, and as Yaacov says, INLINE is actually orthogonal to error behavior - it can be used with any of the modes. So actually maybe we have something like:

enum __ErrorBoundary {
  """Errors should propagate to the root of the operation (effectively aborting the entire request)"""
  ROOT
  """Errors should propagate to the first nullable position"""
  NULLABLE
  """Errors should not propagate"""
  LOCAL
}

enum __ErrorLocation {
  """Errored positions should become `null` and the error should be collected in a separate "errors" list."""
  ERRORS_LIST
  """Errors should be serialized inline, and a separate "errorPaths" list should track where they occurred."""
  INLINE
}

directive @behavior(
  errorBoundary: __ErrorBoundary = NULLABLE
  errorLocation: __ErrorLocation = ERRORS_LIST
) on OPERATION

[martinbonnin]

What is the advantage of using INLINE + errorPaths compared to ERRORS_LIST?

[benjie]

It's more legible when reading the response data as a human/developer. Rather than seeing null and then checking in a separate list to see if that's a "legit null" or and "error null", you can see there's an error inline - and you'd likely recognize the shape of an error (plus we'd probably add something to it that GraphQL couldn't output except in the case of custom scalars). Much more like Result<T> for every value, right?

[benjie]

(The errorPaths would help a GraphQL client to know a) those positions are definitely errors, not custom scalars, and b) to know where the errors occurred without having to walk the response tree.)

[martinbonnin]

It's more legible when reading the response data as a human/developer.

Gotcha, thanks! 👍

we'd probably add something to it that GraphQL couldn't output except in the case of custom scalars

Wondering if we could add a mode where schema authors could say "my custom scalar will never contain __$$error key" and make the errors truely inline:

enum __ErrorLocation {
  """Errored positions should become `null` and the error should be collected in a separate "errors" list."""
  ERRORS_LIST
  """Errors should be serialized inline, and a separate "errorPaths" list should track where they occurred."""
  MOSTLY_INLINE
  """Errors should be serialized inline, custom scalars must not be JSON objects containing a `__$$error` key."""
  INLINE
}

I would really like to see response streaming supported one day to help reduce latency and inlining errors is required for that.

[benjie]

In my experience schemas that accept JSON do minimal validation on it; walking the entire tree to ensure it doesn't contain __$$error at any position at any depth might be asking a bit much. I see no harm in including errorPaths to solve any ambiguities, clients would only need to check it if the position that returned the object was a custom scalar position.

(Also, for things like graphql-toe, it's more performant to be informed where the errors occur and handle just those positions rather than having to walk the entire response tree to look for them before allowing the client to render.)

[martinbonnin]

I see no harm in including errorPaths to solve any ambiguities,

My whole grudge with errorPaths (and errors) is that for an efficient implementation on the client, errorPath needs to be first in the response JSON. If not, the client has to buffer the whole response in memory before it can parse the error positions.

And making errorPath first is sub-optimal for the server because it means the server now needs to buffer the whole response. So I don't see a way out without a true INLINE errors location.

It can be argued that with parallel execution it's already the case that the server needs to buffer a little bit but it could stil be optimized for mutations and for the queries where the slow field happens later in the query so this is something that'd be cool to have regardless.

[benjie]

Counterproposal: this should be a request option not a directive: graphql/nullability-wg#86