Skip to main content

Sibling errors should not be added after propagation

At a glance​

Timeline​

This PR is built on top of:

  • #1183

GraphQL.js output is not (currently) stable after an operation terminates: more errors may be added to the result after the promise has resolved!

Reproduction with graphql module test.mts
import type { ExecutionResult } from "graphql";
import {
  graphql,
  GraphQLInt,
  GraphQLNonNull,
  GraphQLObjectType,
  GraphQLSchema,
} from "graphql";

const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

const Test = new GraphQLObjectType({
  name: "Test",
  fields: {
    a: {
      type: GraphQLInt,
      async resolve() {
        await sleep(0);
        throw new Error(`a`);
      },
    },
    b: {
      type: new GraphQLNonNull(GraphQLInt),
      async resolve() {
        await sleep(10);
        throw new Error(`b`);
      },
    },
    c: {
      type: GraphQLInt,
      async resolve() {
        await sleep(20);
        throw new Error(`c`);
      },
    },
  },
});

const Query = new GraphQLObjectType({
  name: "Query",
  fields: {
    test: {
      type: Test,
      resolve() {
        return {};
      },
    },
  },
});
const schema = new GraphQLSchema({
  query: Query,
});

const result = await graphql({
  schema,
  source: `{ test { a b c } }`,
});

console.log("Result:");
console.log();
console.log(JSON.stringify(result, null, 2));
await sleep(100);
console.log();
console.log("Exact same object 100ms later:");
console.log();
console.log(JSON.stringify(result, null, 2));
$ node test.mts 
Result:

{
  "errors": [
    { "message": "a", "path": ["test", "a"] },
    { "message": "b", "path": ["test", "b"] }
  ],
  "data": { "test": null }
}

Exact same object 100ms later:

{
  "errors": [
    { "message": "a", "path": ["test", "a"] },
    { "message": "b", "path": ["test", "b"] },
    { "message": "c", "path": ["test", "c"] }
  ],
  "data": { "test": null }
}

(I've formatted this output for brevity)

The reason for this: though we note in the spec that you may cancel sibling execution positions, we don't do that in GraphQL.js; and furthermore, we even process errors from the result and add them to the errors list!

This is particularly problematic for client-side "throw on error". Given this schema:

type Query {
  test: Test
}
type Test {
  a: Int  # Throws immediately
  b: Int! # Throws after 10ms
  c: Int  # Throws after 20ms
}

And the same spec-valid result as above:

{
  "errors": [
    { "message": "a", "path": ["test", "a"] },
    { "message": "b", "path": ["test", "b"] },
    { "message": "c", "path": ["test", "c"] }
  ],
  "data": { "test": null }
}

Technically the Test.b field is the field that caused data.test to be null - it's non-nullable, so it triggered error propagation - but without looking at the schema we can't determine this.

Solution: recommend that servers don't keep adding to errors after error propagation has occurred. This would mean:

  1. GraphQL.js won't keep adding to errors after the operation has "completed"
  2. We can throw the last error received that relates to the associated field, and trust that for an implementation following the recommendations it's going to be the one either from the field itself or from the field that triggered error propagation to this level.