apollographql / apollo-server

🌍  Spec-compliant and production ready JavaScript GraphQL server that lets you develop in a schema-first way. Built for Express, Connect, Hapi, Koa, and more.
https://www.apollographql.com/docs/apollo-server/
MIT License
13.76k stars 2.03k forks source link

Inconsistent response between required and optional fields #7235

Closed AndreasHald closed 1 year ago

AndreasHald commented 1 year ago

Issue Description

Querying fields that is required / not required provides inconsistent responses.

Consider the repro - there are 2 fields that can be queried willFailFieldIsRequired and willFailFieldIsNotRequired both return a string and the only difference is that one field is required and the other is not. the resolvers for both fields throws an error meaning that both will fail. However the shape of the response differs.

willFailFieldIsRequired returns the expected errors array with the data property as null

  errors: [
    {
      message: 'something went wrong',
      locations: [Array],
      path: [Array],
      extensions: [Object]
    }
  ],
  data: null
}

willFailFieldIsNotRequired returns the expected errors array, but the data property is an object with the key willFailFieldIsNotRequired as null.

{
  errors: [
    {
      message: 'something went wrong',
      locations: [Array],
      path: [Array],
      extensions: [Object]
    }
  ],
  data: { willFailFieldIsNotRequired: null }
}

Other than the fact that this is inconsistent it causes issues with apollo client, because querying a field that is required if the resolver throws an error the Apollo client fails with null is not an object (evaluating 'rootValue[aliasedFieldName]') because the clients expects the aliasedFieldName to be present within data. And it only is if the field is optional.

This essentially means it is impossible to have any required field be able to throw an error.

Link to Reproduction

https://stackblitz.com/edit/node-datsna?file=index.mjs

Reproduction Steps

glasser commented 1 year ago

This "null bubbling" property is part of the GraphQL specification (http://spec.graphql.org/draft/#sec-Handling-Field-Errors). It allows you to rely on the fact that if a non-null field is requested, it will never show up as null. If you want to be able to get "partial" responses where a particular subtree is optional, then an appropriate way to create that "error boundary" is to declare the field as nullable.

There is an RFC under moderately active development (https://github.com/graphql/graphql-spec/pull/895, https://github.com/graphql/graphql-wg/blob/main/rfcs/ClientControlledNullability.md) that would allow clients to treat a non-null field as nullable for (among other things) the sake of this logic. You may want to chime in there! Apollo Server's execution is defined by graphql-js, the reference implementation of GraphQL, so this behavior won't change unilaterally here.

AndreasHald commented 1 year ago

@glasser thanks for the well thought through response. I understand you won't and shouldn't differ from graphl-js

However this causes an internal error from apollo client when a required field throws an error. And that can't be intendee behavior