CCN’s ? can save normalized caches from null bubbling corruption

[captbaritone]

TL;DR: I believe smart clients like Relay, which maintain a normalized cache, could leverage CCN’s ? to prevent cache corruption due to null bubbling

The Problem

Null bubbling (when a error or null value encountered in a non-nullable field bubbles up to a parent nullable field) is destructive. It causes true and correct data that could be included in the response to be omitted. This can cause problems when trying to write a GraphQL response into a normalized cache.

Imagine two cards A and B side by side, each of which contain information about the current user. A shows “name” and B shows “age”. They are powered by two different queries. The response for query A looks like this:

{ // Response A
  "data": {
    "me": { "id": "10", "name": "Jordan}
  }
}

We can write this into our normalized cache and it will look like:

{ // Normalized cache
  "ROOT": {
    "me": {__id: "10"}
  },
  "10": {
    "id": "10",
    "name": "Jordan"
  }
}

This is fine, and we can nicely render our first card.

Now, however, we get an error in our second query’s response:

{ // Response B
  "data": {
    "me": null // <-- Null due to bubbling
  }
  "errors": [{ "message": "Age errored", "path": ["me", "age"] }]
}

The age field, which was non-nullable, has errored, and we won’t be able to render our second card. Oh well, such is life. However, when we go to write this into our cache, things get worse:

{ // Normalized cache
  "ROOT": {
    "me": null // <-- We had to write null here!
  },
  "10": {
    "id": "10",
    "name": "Jordan"
  }
}

Now card A, which also reads from this normalized store, is broken despite the fact that none of the fields it reads are in an error state.

A possible solution

With the adoption of Relay’s proposed error handling feature, product code is shielded from implicit field errors via framework-level explicit error handling. This means Relay is no-longer dependent on the server’s response shape strictly matching the schema. Specifically, it’s fine for a field marked as an error to be missing, even if it’s non-nullable. In other words, Relay can safely opt-out of server-side null bubbling.

And CCN's ? offers Relay just that option. Relay’s compiler can annotate every non-null field in the query it generates with a ? to opt out of any null bubbling.

Note: I’ve documented the CCN behavior I’m expecting here.

A new world

Lets now imagine how response B would look like in this new world:

{ // Response B (without null bubbling)
  "data": {
    "me": {
      "id": "10",
      "age": null // <-- Errored, but did not bubble!
    }
  }
  "errors": [{
    "message": "Age errored",
    "path": ["me", "age"]
  }]
}

When we write this into our normalized cache we get:

{ // Normalized cache
  "ROOT": {
    "me": {__id: "10"}
  },
  "10": {
    "id": "10",
    "name": "Jordan"
    "age": Error("Age errored")
  }
}

Card B still can’t render, but note that we’ve prevented our normalized cache from getting corrupted due to null bubbling. Thanks CCN!

Related

This post is spiritually related to #19 in that they both explore the benefits of a mode of GraphQL execution that avoids null bubbling. It may be worth exploring other mechanisms where-by clients could opt out of null bubbling, but I wanted to point out that CCN’s ? is a powerful enough primitive to allow a smart client like Relay to opt out of null bubbling.

---

Hat tip to @RyanHoldren who wrote an excellent internal post articulating this normalization issue. My note here is very much inspired by that post.

[benjie]

It would be wise for Relay to not only add ? to every non-null field, but to nullable fields to in case the schema evolves (since changing a nullable output field to non-nullable is generally seen as non-breaking). Perhaps a shortcut could be to place a ? after the operation name: query Foo? {…}

[captbaritone]

Good point about schema changes! Ultimately what a client like Relay wants, both for the purpose outlined in this post as well as the True Nullability Schema goal outlined in #19, is an option to opt out of null bubbling globally, exactly as you propose.

A global flag like this more elegantly describes what Relay, and other sufficiently-smart normalizing clients, wants. In addition a global flag would be approachable for simpler clients that don't transform a user-defined query. Although, for those middle-tier smart-clients that don't transform, maybe they would prefer that the flag be supplied as a header or http query variable. That way the client could select the execution mode without needing the user's participation (adding the query-level annotation)...

I'm was a bit hesitant to propose a change where a top-level annotation forks execution behavior for the full query. Having two separate execution modes sounds undesirable from the perspective of the spec and also the mental overhead of the whole ecosystem. That said, I think the value that could be unlocked by a true nullability schema is considerable and would likely be worth the added complexity.

I think the big question here is:

For smart clients that can handle errors client-side, a single top-level annotation like you propose would obviate the need for the rest of CCN. Should we consider skipping CCN, with its complexity, limitations, and tradeoffs, all together and try to make the jump straight to True Schema Nullability?

Some outstanding questions are:

1. How do simple clients stay resilient in the face of a true nullability schema? (Does it matter?)
2. How practical is it to migrate to a true nullability schema? Legacy clients (apps that have been deployed) will be retroactively opted into a less resilient execution mode.

[benjie]

Question: in an ideal world, would Relay have GraphQL inline the error rather than returning it in a separate errors list? I'm thinking:

{ // Response B (without null bubbling)
  "data": {
    "me": {
      "id": "10",
      "age": {
        $$isError: true, // <-- note that this key is not possible through aliases, but could happen in custom scalars... Risky.
        "message": "Age errored",
        // no need for "path"
      } // <-- Errored, but did not bubble!
    }
  }
}

Answer: we can't do this, because of custom scalars. If your JSON field errored, there's no way for the client to differentiate this between an actual error and data that happens to contain $$isError: true.

[captbaritone]

in an ideal world, would Relay have GraphQL inline the error rather than returning it in a separate errors list?

I see that you answered your own question regarding that particular encoding scheme, but to answer your question more broadly, I think you're spot on. With explicit error handling, Relay is going to start thinking of the response as a single tree that just happens to encode errors in a look-aside array for the purposes of unambiguous JSON encoding. Relay's runtime is sufficiently sophisticated to be able to, in essence, perform that transformation on the client.

So, for us, the only advantage of a server response that actually was a single tree, would just be some slight performance advantages when interpreting the server response.

[martinbonnin]

So, for us, the only advantage of a server response that actually was a single tree, would just be some slight performance advantages when interpreting the server response.

Same for Apollo Kotlin. As with everything performance related it'd have to be benchmarked but from the top of my mind, getting errors in a separate tree causes a hash map lookup for each null field or so (to know if this null field is indeed an error or a "true" null). Maybe it's not too bad, especially if it can be amortised during network I/O.

Overall JSON in itself is not the best in terms of performance so I'd say this is probably acceptable. Any performance-sensitive app could switch to another wire format that can encode errors inline and be parsed faster.

[benjie]

Same for almost any other JS client with GraphQL TOE (Throw On Error) which throws when a position that produced an error is accessed.

[martinbonnin]

There's been a lot of discussions and I'd like to consolidate the current state of discussions in https://github.com/graphql/graphql-wg/blob/main/rfcs/SemanticNullability.md. Can we close this discussion? It'll still be there and can be reopened if need but it's an incentive to centralise the discussions as we move towards an the end goal.

[benjie]

This is one of the key discussions that led to disabling null bubbling becoming the aim. That doesn’t mean we shouldn’t close it, but it’s a valuable discussion we should ensure is linked as motivation in the relevant places. I’ll go add it to my request option PR 👍

[martinbonnin]

I'd still like to close it in order for:

• the discussion to happen in a smaller, more recent set of issues/discussions
• the newer issues/discussions being surfaced more preminently for people new to the topic and doing a search in the repo.

But definitely +1 to keep it around for reference/historical documentation purposes. Linking here for context is definitely useful!