Is null propagation still desirable behavior?

[twof]

I want to say up front that CCN should absolutely not be blocked on this discussion.

During our work on CCN, we've run into quite a few holes in GQL, error handling, fragment modularity, feature introspection, etc, but I'm not sure we've ever asked the most base question: Is null propagation still desirable in 2023?

From the spec:

If a field error is raised while resolving a field, it is handled as though the field returned null, and the error must be added to the "errors" list in the response.

If the result of resolving a field is null (either because the function to resolve the field returned null or because a field error was raised), and that field is of a Non-Null type, then a field error is raised. The error must be added to the "errors" list in the response.

Since Non-Null type fields cannot be null, field errors are propagated to be handled by the parent field. If the parent field may be null then it resolves to null, otherwise if it is a Non-Null type, the field error is further propagated to its parent field.

I basically would like to float the possibility that rather than creating the a feature (CCN) to adapt to null propagation, null propagation itself could be removed or altered so it isn't so problematic.

Please discuss the reason for your answer in the comments.

[martinbonnin]

I voted "no" cause null propagation feels a bit awkward. Using null for both actual values and errors feels dangerous. I'm curious if there has ever been talks about using "undefined" for errors instead?

{
  "data": {
    "hero": {
      "name": "Luke"
      // no height here because the height field threw an error
    }
  },
  "errors": [
    {
      "message": "Cannot fetch height",
      "path": ["hero", "height"]
    }
  ]
}

Then there's no risk of cache poisoning.

[twof]

As we've learned from CCN discussions, null propagation has far reaching effects. The official recommendation is to rarely make fields Non-Nullable, basically dissuading users from using a feature of the language.

Relay has a very long history of dealing with null propagation by basically operating outside of the GraphQL spec with a facade of fragment isolation along with several other adaptations to avoid blowing out their normalized cache. They, along with the Kotlin Apollo client, and Netflix each independently introduced directives that act something like CCN, and now they're working on @catch.

Here is a recent writeup about how CCN can be used to prevent the destructive impact of null propagation on Relay's caches: graphql/nullability-wg#20

CCN has been in the works for well over 2 years now, a lot of that time spent trying to work through all of the subtle ways null propagation can cause issues for clients. It's not clear to me for what use case null propagation is desirable behavior. If it wouldn't be so horrendously breaking, I'd suggest that null propagation be re-evaluated and removed in the next major version of GraphQL. An opt-in experimental feature might be more viable.

Possible non-destructive replacement behavior:

If a field error is raised while resolving a field, it is handled as though the field returned null, and the error must be added to the "errors" list in the response.

If the result of resolving a field is null (either because the function to resolve the field returned null or because a field error was raised), and that field is of a Non-Null type, then a field error is raised. The error must be added to the "errors" list in the response. That error must contain a field "__code__" set to 0 in order to allow developers to identify that this error was the result of a Non-Null field.

Since Non-Null type fields cannot be null, field errors are propagated to be handled by the parent field. If the parent field may be null then it resolves to null, otherwise if it is a Non-Null type, the field error is further propagated to its parent field.

[captbaritone]

I'd second what @benjie said here. curl is just an example of a simple client, but that category includes window.fetch as well as Python's Requests and any other languages "I'd like to make an HTTP request please" API.

[benjie]

The fact that there's so many open examples of people using curl for GraphQL is super interesting though!

[captbaritone]

There were some interesting examples of "simple" clients that came up in our informal CCN meeting today:

Queries used by various scripts and jobs, just as cron jobs or tasks used to build indexes.

[twof]

It was also said that this change in combination with correctly typed schemas could cause errors to be added to the error array that weren't previously there, and these simple clients often throw out everything if there is anything in the errors array. That's something I hadn't considered before.

[benjie]

Honestly i think most simple clients don’t even look at errors. They just use data with safe access patterns (e.g. optional chaining or jq). And in my experience this is even true for the vast majority of small companies using Apollo client, other than for mutations where they explicitly check for errors.

[benjie]

If you were going to start GraphQL today, would you choose the current null propagation behavior over again?

I think if we were to write a new GraphQL 2.0, I would:

1. have fragment identification of some kind
2. have fragments be error boundaries

I think it's critical that GraphQL responses match the types given in the schema, and without indicating these again through the response a schema-unaware client wouldn't know whether to bubble the null or not. Similarly simply omitting errored fields would break the expectation of developers for schema-unaware clients - they've asked for { user { name avatar balance } } and balance errored, so now they're rendering balance: undefined into their apps... Not ideal. It seems to me that omitting the fields is the best way, but it must be done in a "component-aware" way - so that the entire component either renders or doesn't - and the way to do that is through fragments.

One of the most common issues I see with people using GraphQL is people not using fragments. This is something we should all work on improving - encouraging the best practices of using fragments widely and co-locating data requirements with components. I read @jensneuse's post https://wundergraph.com/blog/graphql_doesnt_solve_underfetching_overfetching_but_relay recently and the examples of just how many GraphQL queries these sites are issuing really indicates that we're not educating people enough of how they should use GraphQL. But I'm getting a little off topic, so I'll shut up about that...

[benjie]

Oh also for clarification I don't think that every fragment would be an error boundary. Probably only named fragments, and even then only when they opt in e.g. via ? (or, maybe, unless they opt-out via !). If the fragment failed then it simply wouldn't be merged back into the result (which is "fun" because GraphQL's execution engine doesn't currently care about fragments at all, transforming them into merged selection sets before executing the fields).

[captbaritone]

Fragments becoming error boundaries is exactly what we're exploring for Relay. Of course to support this in the spec without diving the spec into explicit client/server responsibilities we would first need Fragment Modularity.

One thread I'd like to pull on though is "I think it's critical that GraphQL responses match the types given in the schema".

I fully agree, but I wonder if it must be the data portion alone that matches the schema. My current mental model is that we could instead think of the full response (data + errors) as an encoding of a type-safe tree where the erroring fields have been explicitly noted as in an error state. The fact that we encode that single tree using a data tree and an errors array is just an implementation detail to work around the limitations of JSON.

[benjie]

If clients have to do their own transforms to make it type safe then I’d argue it isn’t, unless we specify every client must; which is going to be too much for simple curl/fetch/etc clients. However if clients opt-in to this i think it’s fine.

[martinbonnin]

JSON has no error type which I believe is the underlying problem here. It's a problem of matching the GraphQL type system to the JSON type system and how we define "type-safe". I'm on board with @captbaritone that I would still consider (data + errors) to be type safe here given all the use cases it unlocks.
Or we design GraphQL 2.0 to use something else than JSON...