Write up concerns regarding the try/catch method proposed for CCN

[benjie]

Benjie expressed concerns about the try/catch interpretation of CCN and would prefer to just use the mechanisms that GraphQL already has. He intends to write up some examples to demonstrate the issues that he foresees.

• assignee(s): @benjie
• source: https://github.com/graphql/client-controlled-nullability-wg/blob/main/notes/2023/2023-07.md#first-meeting-july-19th

---

Note: Action Item issues are reviewed and closed during Working Group
meetings.

[benjie]

Relates to this discussion: graphql/graphql-wg#1009 (reply in thread)

[benjie]

Ignoring the orthagonal concerns of where errors are raised in the response and other such things, I think we're broadly discussing that there are three main options for Client Controlled Nullability:

1. Use the existing execution algorithms, but with the type of the field replaced with the nullability-modified version. I'll call this SIMPLE_NULLABILITY
2. Use a try/catch approach where a null at a ! position blows up everything until the next ?. I'll call this TRY_CATCH.
3. As (2), but where everything between the ? and ! have implicit ! (including inside lists), I'll call this TRY_CATCH_IMPLICIT

Up front, I want to state (for the sake of transparency, and to help you carefully analyze the bias in my statements) that my preference continues to be option (1). In my opinion option 1 is both the most straightforward to understand (there's nothing extra to learn), straightforward to implement (all we need is the ability to substitute the type in the existing algorithm), and has the fewest unintended consequences.

Example schema

This schema is used for all examples:

type Query {
  forum(id: ID!): Forum
}

type Forum {
  id: ID!
  posts(first: Int): [Post]
}

type Post {
  id: ID!
  topic: String
  author: User
}

type User {
  id: ID!
  name: String
  avatarUrl: String!
}

Example 1

Consider this query:

query ForumMainPage ($id: ID!) {
  forum(id: $id) {
    posts(first: 10) {
      ...Post
    }
  }
}

fragment Post on Post {
  id
  topic
  author {
    ...Author
  }
}

fragment Author on User {
  id
  avatarUrl
  name!
}

Let's imagine that someone is writing the Author component to render the name and avatar of the user who posted a comment or post or similar. They know that User.name is never null (it's required and validated during signup), and figure its nullability must be a schema modelling mistake (imagine them shaking their fist at GraphQL's "nullable by default" policy), so they add a ! so they don't have to do all the null checks in their code since they know better.

The person responsible for rendering the Post doesn't know or care what's inside the Author component, so they just splat that out without digging too deeply. They may well have written the Post to compose the Author fragment before the ! was added to the Author fragment.

A month passes, and the business realises that they need a way for users to delete their accounts. They don't want to delete the posts, but they make it so that if the author is null then that represents a deleted user. (Had they thought ahead they might have used union Author = User | DeletedUser, but it's too late for that now, the existing schema is already in use on their mobile clients.) Seems reasonable from an API perspective, so they ship it out.

Another month passes, and the business changes the sign up process for users. A User's name is now optional to streamline the onboarding funnel.

Consider a mobile client released before these two API changes, dealing with complexities in the latest data.

a. When rendering the list with a null name

With SIMPLE_NULLABILITY, an author with null name will result in a null author. The author would simply not be rendered from the Post component.

With TRY_CATCH or TRY_CATCH_IMPLICIT, an author with null name will result in the entire query blowing up, since there is no ?.

b. When rendering a list with a null name and null avatarUrl

Since avatarUrl occurs before name!, it will be checked first, and the null avatarUrl will cause the traditional GraphQL error handling to kick in, resulting in author: null. Thus SIMPLE_NULLABILITY and TRY_CATCH would work fine, but TRY_CATCH_IMPLICIT would still blow up the request.

c. When rendering the list with a null author

With SIMPLE_NULLABILITY or TRY_CATCH, a null author simply won't be rendered. In the case of TRY_CATCH the author being null means the name field is never checked, so a null author simply won't be rendered.

With TRY_CATCH_IMPLICIT, the author becomes non-nullable, blowing up the whole query.

Summary of concerns

The outcomes of (1.a) and (1.b) demonstrate the inconsistency and confusing nature of TRY_CATCH, and to my mind rule it out for this reason.

With TRY_CATCH_IMPLICIT, any page that renders a post with a null author or a post with an author with no name will now no longer function. The app is essentially rendered useless whenever a post from a deleted user is returned. This may not be the desired outcome from marking the name as non-nullable - I don't think users would expect this to require that the author itself exists, and adding the ? to handle it would involve crossing fragment boundaries (and thus, potentially, teams).

Example 2

The same as Example 1, but with an error boundary added at forum>posts:

query ForumMainPage ($id: ID!) {
  forum(id: $id) {
    posts(first: 10)[?] {
      ...Post
    }
  }
}

When rendering the list with a null name

With SIMPLE_NULLABILITY, an author with null name will result in a null author. The Post component would render, but the author would not.

With TRY_CATCH_IMPLICIT, an author with null name will result in the post becoming null (and thus not being rendered) - an acceptable outcome.

With TRY_CATCH, if avatarUrl throws then we'll get the same outcome as SIMPLE_NULLABILITY. If avatarUrl does not throw we'll get the same outcome as TRY_CATCH_IMPLICIT.

When rendering the list with a null author

With SIMPLE_NULLABILITY or TRY_CATCH, the post will be rendered but a null author won't be.

With TRY_CATCH_IMPLICIT, the author becomes non-nullable, blowing up the entire post.

Summary of concerns

In the TRY_CATCH approach, having a null-name results in the post being skipped, but having a null author (through error or explicit null) results in the Post component rendering without the Author. If it can render without an author, why can't it render without an authors name? This inconsistency seems unexpected, and is what I argued against in graphql/graphql-wg#1009

Additional comments

TRY_CATCH_IMPLICIT would require the person writing the query to reiterate ? on any nullable field where the null can be handled, such as author. This is a lot of work for the person issuing the query or writing/composing the fragments, and without tooling to help is liable to go wrong (especially on list fields). I would advise my team to avoid this feature for this reason, or at least to use it very sparingly.

TRY_CATCH leads to inconsistencies that could be confusing. I don't see it as a tenable solution, at least how it's currently proposed.

Further, in general the behavior of blowing up the entire response when the Author fragment fails feels questionable to me. Skipping the Author or the Post would seem better than all-out failure. If we go for this approach, we may have to strongly encourage users to use ? heavily.

I, personally, am unconvinced that TRY_CATCH or TRY_CATCH_IMPLICIT are suitable for general usage unless we introduce fragments as error boundaries. With fragments as error boundaries I think that they could certainly become much more compelling (and we could also lift the validation requirements that all fields at the same operation path have the same nullability specifiers), but I'm still unconvinced in the value of adopting either of these approaches over SIMPLE_NULLABILITY.

Further, I think it's much more obvious to a user to add ! at every level if this "blow up the entire request if any of the post authors don't have a name" is what they intend, rather than having them add ? to only the nullable places (and having to look these up).