Error propagation and "following" field executions.

[ddebrunner]

I understand the guidance for nullable fields due to error propagation (e.g. see #719) but wondering what the spec defines (or maybe should define) when propagation does occur.

With a mutation operation such as:

mutation {m1 m2 m3}

then assume m2 had an error and was non-nullable, the response should be:

{
   "data": null
   "errors": [{"path":["m2"], "message": "some error"}]
}

The question is, should m3 be executed?

Maybe a strict reading of the spec would indicate m3 must be executed, and the only way the client can know that it succeeded, is that there is no error for m3. So m1 and m3 were executed, both without errors, but the client has no ability to determine their values.

For query operations, I would assume there is more freedom for the implementing engine, as operations are defined to be idempotent.
So a similar:

query {q1 {q1a} q2 q3 {q3a} }

could avoid executing q1a, q3a if (non-nullable) q2 failed, as any values it provides will not be returned.
[obviously this is subject to timing based upon parallel execution, but can the engine avoid executing fields whose values can no longer be returned].

Maybe, it's all left as an choice for the implementing engine, and in the case of mutation, the response could be:

{
   "data": null
   "errors": [
       {"path":["m2"], "message": "some error"}
       {"path":["m3"], "message": "canceled"}
   ]
}

Wondering if there had been any discussion along these lines?

[benjie]

mutation {m1 m2 m3}

In general, all root-level fields should be nullable, so this should result in:

{
  data: {
    m1: { /* ... */ },
    m2: null,
    m3: { /* ... */ },
  },
  errors: [
    {path: ["m2"], /* ... */}
  ]
}

But for the purposes of your question I think you're assuming m2 is non-nullable. In which case we know m1, m2 and m3 must run in serial, we know m1 passes and m2 fails, we know m2's null will be bubbled to the root, but I think it's up to the implementation whether or not m3 starts. I would expect the vast majority of implementations to not start m3, as noted:

If during ExecuteCollectedFields() a response position with a non-null type raises an execution error then that error must propagate to the parent response position [...] If this occurs, any sibling response positions which have not yet executed or have not yet yielded a value may be cancelled to avoid unnecessary work.
-- https://spec.graphql.org/draft/#sec-Executing-Collected-Fields.Errors-and-Non-Null-Types

Also because the most obvious way of handling errors in GraphQL is with try/catch-style mechanics where non-nullable positions re-throw, and this would result in m3 never being called in the first place.

With error propagation disabled (e.g. via @experimental_disableErrorPropagation or onError: NULL (see #1163)), all three fields will execute.

HTH

[benjie]

in the case of mutation, the response could be: [...]

UPDATE: This is incorrect!

Just to clarify this is explicitly forbidden by the spec - each response position that's null may only have one associated error (which may come from a deeper path). That response only has one response position (data: null) so it may only contain one error.

Click for a demonstration with graphql.js

import {
  graphql,
  GraphQLInt,
  GraphQLNonNull,
  GraphQLObjectType,
  GraphQLSchema,
} from './npmDist/index.mjs';

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

const mkField = (counter: number) => ({
  type: new GraphQLNonNull(GraphQLInt),
  async resolve() {
    console.log(`Triggered ${counter}`);
    await sleep(Math.random() * 10);
    console.log(`Resolved ${counter}`);
    throw new Error(`E${counter}`);
  },
});

const Query = new GraphQLObjectType({
  name: 'Query',
  fields: {
    a: mkField(1),
    b: mkField(2),
    c: mkField(3),
  },
});
const Mutation = new GraphQLObjectType({
  ...Query.toConfig(),
  name: 'Mutation',
});
const schema = new GraphQLSchema({
  query: Query,
  mutation: Mutation,
});

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

console.dir(result);

(Node 24+)

$ node test.mts 
Triggered 1
Resolved 1
{
  data: null,
  errors: [
    Error: E1
        at resolve (file:///home/benjie/Dev/graphql/graphql-js/test.mts:17:11)
        at async completePromisedValue (file:///home/benjie/Dev/graphql/graphql-js/npmDist/execution/execute.mjs:564:26)
        at async file:///home/benjie/Dev/graphql/graphql-js/test.mts:38:16 {
      path: [Array],
      locations: [Array],
      extensions: [Object: null prototype] {}
    }
  ]
}

[ddebrunner]

@benjie Thanks I missed the cancel statement because I was looking in https://spec.graphql.org/October2021/#sec-Handling-Field-Errors

[ddebrunner]

That response only has one response position (data: null) so it may only contain one error.

@benjie Not sure the spec says that, it says only one error should be added to errors per response position.

For example, if m2 was executing and there was an error at path [m2, a, b] that was nullable, then there is an error with path [m2, a, b].

Now if later path [m2, c] has an error and is non-nullable up to the root, then m2 becomes null due to the error at path [m2, c] which has already been added into errors.

So in that case, what does errors contain?

[
{"path":["m2", "a", "b"], "message": "nullable error"}
{"path":["m2", "c"], "message": "non-nullable error"}
]

The spec says those two errors must be added to errors

So in the m1,m2,m3 case, if m2 failed and propagated null to the top level then if nullable m1 or m3 had errors then the response must contain them.

[benjie]

I believe this is a bug/ambiguity in the spec wording and have raised a fix here:

#1183

[ddebrunner]

Closing, my original issue is covered by "a value may be cancelled to avoid unnecessary work." and the separate issue of which errors should be present can be discussed in #1183.

Thanks @benjie

[benjie]

Thanks for raising the issue, it sparked some external discussions too from other contributors who were not clear on this, so it helped me prioritize clarifying it!

[benjie]

As noted in the related PR, my understanding wasn't quite correct and nullable positions can have their errors retained after non-nullable positions cause a bubble to occur. I've updated #1183 to reflect this.