Remove non-nullable in many types in graph-ql/coverage/company.md

[DrewML]

Problem

We're using a lot of non-null types in the Company GraphQL Schema. I don't think this is an ideal starting place.

From the GraphQL specification:

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 it’s parent field.

http://spec.graphql.org/June2018/#sec-Errors-and-Non-Nullability

In other words, a non-null type infects things up the tree until it finds a non-null field.

Take the following simplified example from our company schema:

Schema

type Query {
   company: Company
}

type Company {
	id: ID!
    name: String!
    legal_address: CompanyLegalAddress!
    users: CompanyUsers
}

type CompanyLegalAddress {
   city: String!
}

Client Query

query GetCompanyInfo {
   company {
      id
      name
      legal_address {
         city
      }
      users {
         firstname
         lastname
         job_title
      }
   }
}

When executing this query, imagine that the Company.CompanyLegalAddress resolver calls an external AddressService, and that service has a failure. This is the response the client will get:

{
   "data": {
      "company": null
   }
}

Uh oh. Company.CompanyLegalAddress.city was marked as non-nullable, but it couldn't fetch its data. That means that the GraphQL executor is going to walk up to the next parent. But Company.legal_address is also a non-null. For this query, the closest non-null is Query.company, so the client will not get any data it requested.

If Company.legal_address was a nullable type, the response would have looked more like this:

{
   "data": {
      "company": {
         "id": 1,
         "name": "Some name",
         "legal_address": null,
         "users": [{ "firstname": "foo", "lastname": "bar", "job_title": "marketing" }]
      }
   }
}

As we move towards a distributed architecture, and use GraphQL to stitch all those sources of data together, it's important that our schema design enables clients to gracefully handle outages of some services.

Backwards Compatibility

It's a backwards-compatible change to the schema if we want to make some fields non-nullable in the future. That does not break any guarantees to the client.

However, switching from nullable to non-null is a breaking change. Given our goals are BIC changes in GraphQL, it's safer to use nullables when in doubt.

Solution

Remove non-nullable types from the majority of GraphQLObjectTypes in the company schema.

Leave non-null fields only where they would be critical for any nested operations to succeed. Example: If we can't lookup an ID for a resource, we know the queries below are going to fail, so we can fail early.

Further Reading

• Older blog post about non-null types propagating upwards
• Background explaining why nullable fields are the default

Requested Reviewers

[kokoc]

I think the recommendation from PR description should be provided for every schema and placed somewhere in general documents about graphql