Sibling errors should not be added after propagation

src/execution/__tests__/nonnull-test.ts

@@ -2,6 +2,9 @@ import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { expectJSON } from '../../__testUtils__/expectJSON';
+import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick';
+
+import { invariant } from '../../jsutils/invariant';
 
 import { parse } from '../../language/parser';
 
@@ -524,6 +527,143 @@ describe('Execute: handles non-nullable types', () => {
     });
   });
 
+  describe('Handles multiple errors for a single response position', () => {
+    it('nullable and non-nullable root fields throw nested errors', async () => {
+      const query = `
+        {
+          promiseNonNullNest {
+            syncNonNull
+          }
+          promiseNest {
+            syncNonNull
+          }
+        }
+      `;
+      const result = await executeQuery(query, throwingData);
+
+      expectJSON(result).toDeepEqual({
+        data: null,
+        errors: [
+          {
+            message: syncNonNullError.message,
+            path: ['promiseNest', 'syncNonNull'],
+            locations: [{ line: 7, column: 13 }],
+          },
+          {
+            message: syncNonNullError.message,
+            path: ['promiseNonNullNest', 'syncNonNull'],
+            locations: [{ line: 4, column: 13 }],
+          },
+        ],
+      });
+    });
+
+    it('a nullable root field throws a slower nested error after a non-nullable root field throws a nested error', async () => {
+      const query = `
+        {
+          promiseNonNullNest {
+            syncNonNull
+          }
+          promiseNest {
+            promiseNonNull
+          }
+        }
+      `;
+      const result = await executeQuery(query, throwingData);
+
+      expectJSON(result).toDeepEqual({
+        data: null,
+        errors: [
+          {
+            message: syncNonNullError.message,
+            path: ['promiseNonNullNest', 'syncNonNull'],
+            locations: [{ line: 4, column: 13 }],
+          },
+        ],
+      });
+
+      // allow time for slower error to reject
+      invariant(result.errors !== undefined);
+      const initialErrors = [...result.errors];
+      for (let i = 0; i < 5; i++) {
+        // eslint-disable-next-line no-await-in-loop
+        await resolveOnNextTick();
+      }
+      expectJSON(initialErrors).toDeepEqual(result.errors);
+    });
+
+    it('nullable and non-nullable nested fields throw nested errors', async () => {
+      const query = `
+        {
+          syncNest {
+            promiseNonNullNest {
+              syncNonNull
+            }
+            promiseNest {
+              syncNonNull
+            }
+          }
+        }
+      `;
+      const result = await executeQuery(query, throwingData);
+
+      expectJSON(result).toDeepEqual({
+        data: { syncNest: null },
+        errors: [
+          {
+            message: syncNonNullError.message,
+            path: ['syncNest', 'promiseNest', 'syncNonNull'],
+            locations: [{ line: 8, column: 15 }],
+          },
+          {
+            message: syncNonNullError.message,
+            path: ['syncNest', 'promiseNonNullNest', 'syncNonNull'],
+            locations: [{ line: 5, column: 15 }],
+          },
+        ],
+      });
+    });
+
+    it('a nullable nested field throws a slower nested error after a non-nullable nested field throws a nested error', async () => {
+      const query = `
+        {
+          syncNest {
+            promiseNonNullNest {
+              syncNonNull
+            }
+            promiseNest {
+              promiseNest {
+                promiseNest {
+                  promiseNonNull
+                }
+              }
+            }
+          }
+        }
+      `;
+      const result = await executeQuery(query, throwingData);
+
+      expectJSON(result).toDeepEqual({
+        data: { syncNest: null },
+        errors: [
+          {
+            message: syncNonNullError.message,
+            path: ['syncNest', 'promiseNonNullNest', 'syncNonNull'],
+            locations: [{ line: 5, column: 15 }],
+          },
+        ],
+      });
+
+      invariant(result.errors !== undefined);
+      const initialErrors = [...result.errors];
+      for (let i = 0; i < 20; i++) {
+        // eslint-disable-next-line no-await-in-loop
+        await resolveOnNextTick();
+      }
+      expectJSON(initialErrors).toDeepEqual(result.errors);
+    });
+  });
+
   describe('Handles non-null argument', () => {
     const schemaWithNonNullArg = new GraphQLSchema({
       query: new GraphQLObjectType({

src/execution/execute.ts

@@ -114,6 +114,7 @@ export interface ExecutionContext {
   fieldResolver: GraphQLFieldResolver<any, any>;
   typeResolver: GraphQLTypeResolver<any, any>;
   subscribeFieldResolver: GraphQLFieldResolver<any, any>;
+  errorPositions: Set<Path | undefined>;
   errors: Array<GraphQLError>;
 }
 
@@ -208,15 +209,19 @@ export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
       return result.then(
         (data) => buildResponse(data, exeContext.errors),
         (error) => {
-          exeContext.errors.push(error);
-          return buildResponse(null, exeContext.errors);
+          const { errorPositions, errors } = exeContext;
+          errorPositions.add(undefined);
+          errors.push(error);
+          return buildResponse(null, errors);
         },
       );
     }
     return buildResponse(result, exeContext.errors);
   } catch (error) {
-    exeContext.errors.push(error);
-    return buildResponse(null, exeContext.errors);
+    const { errorPositions, errors } = exeContext;
+    errorPositions.add(undefined);
+    errors.push(error);
+    return buildResponse(null, errors);
   }
 }
 
@@ -352,6 +357,7 @@ export function buildExecutionContext(
     fieldResolver: fieldResolver ?? defaultFieldResolver,
     typeResolver: typeResolver ?? defaultTypeResolver,
     subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
+    errorPositions: new Set(),
     errors: [],
   };
 }
@@ -558,13 +564,13 @@ function executeField(
       // to take a second callback for the error case.
       return completed.then(undefined, (rawError) => {
         const error = locatedError(rawError, fieldNodes, pathToArray(path));
-        return handleFieldError(error, returnType, exeContext);
+        return handleFieldError(error, returnType, path, exeContext);
       });
     }
     return completed;
   } catch (rawError) {
     const error = locatedError(rawError, fieldNodes, pathToArray(path));
-    return handleFieldError(error, returnType, exeContext);
+    return handleFieldError(error, returnType, path, exeContext);
   }
 }
 
@@ -597,6 +603,7 @@ export function buildResolveInfo(
 function handleFieldError(
   error: GraphQLError,
   returnType: GraphQLOutputType,
+  path: Path,
   exeContext: ExecutionContext,
 ): null {
   // If the field type is non-nullable, then it is resolved without any
@@ -605,12 +612,32 @@ function handleFieldError(
     throw error;
   }
 
+  // Do not modify errors list if a response position for this error has already been nulled.
+  const errorPositions = exeContext.errorPositions;
+  if (hasNulledPosition(errorPositions, path)) {
+    return null;
+  }
+  errorPositions.add(path);
+
   // Otherwise, error protection is applied, logging the error and resolving
   // a null value for this field if one is encountered.
   exeContext.errors.push(error);
   return null;
 }
 
+function hasNulledPosition(
+  errorPositions: Set<Path | undefined>,
+  path: Path | undefined,
+): boolean {
+  if (errorPositions.has(path)) {
+    return true;
+  }
+  if (path === undefined) {
+    return false;
+  }
+  return hasNulledPosition(errorPositions, path.prev);
+}
+
 /**
  * Implements the instructions for completeValue as defined in the
  * "Value Completion" section of the spec.
@@ -779,13 +806,13 @@ function completeListValue(
             fieldNodes,
             pathToArray(itemPath),
           );
-          return handleFieldError(error, itemType, exeContext);
+          return handleFieldError(error, itemType, itemPath, exeContext);
         });
       }
       return completedItem;
     } catch (rawError) {
       const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
-      return handleFieldError(error, itemType, exeContext);
+      return handleFieldError(error, itemType, itemPath, exeContext);
     }
   });