stop resolvers after execution ends (#4263)

depends on #4267

addresses: #3792

Author: yaacovCR

src/execution/__tests__/nonnull-test.ts

@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { expectJSON } from '../../__testUtils__/expectJSON.js';
+import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
 
 import type { PromiseOrValue } from '../../jsutils/PromiseOrValue.js';
 
@@ -526,6 +527,68 @@ describe('Execute: handles non-nullable types', () => {
     });
   });
 
+  describe('cancellation with null bubbling', () => {
+    function nestedPromise(n: number): string {
+      return n > 0 ? `promiseNest { ${nestedPromise(n - 1)} }` : 'promise';
+    }
+
+    it('returns an single error without cancellation', async () => {
+      const query = `
+        {
+          promiseNonNull,
+          ${nestedPromise(4)}
+        }
+      `;
+
+      const result = await executeQuery(query, throwingData);
+      expectJSON(result).toDeepEqual({
+        data: null,
+        errors: [
+          // does not include syncNullError because result returns prior to it being added
+          {
+            message: 'promiseNonNull',
+            path: ['promiseNonNull'],
+            locations: [{ line: 3, column: 11 }],
+          },
+        ],
+      });
+    });
+
+    it('stops running despite error', async () => {
+      const query = `
+        {
+          promiseNonNull,
+          ${nestedPromise(10)}
+        }
+      `;
+
+      let counter = 0;
+      const rootValue = {
+        ...throwingData,
+        promiseNest() {
+          return new Promise((resolve) => {
+            counter++;
+            resolve(rootValue);
+          });
+        },
+      };
+      const result = await executeQuery(query, rootValue);
+      expectJSON(result).toDeepEqual({
+        data: null,
+        errors: [
+          {
+            message: 'promiseNonNull',
+            path: ['promiseNonNull'],
+            locations: [{ line: 3, column: 11 }],
+          },
+        ],
+      });
+      const counterAtExecutionEnd = counter;
+      await resolveOnNextTick();
+      expect(counter).to.equal(counterAtExecutionEnd);
+    });
+  });
+
   describe('Handles non-null argument', () => {
     const schemaWithNonNullArg = new GraphQLSchema({
       query: new GraphQLObjectType({

src/execution/execute.ts

@@ -145,6 +145,7 @@ export interface ExecutionContext {
   validatedExecutionArgs: ValidatedExecutionArgs;
   errors: Array<GraphQLError>;
   promiseCanceller: PromiseCanceller | undefined;
+  completed: boolean;
 }
 
 /**
@@ -244,6 +245,7 @@ export function executeQueryOrMutationOrSubscriptionEvent(
     promiseCanceller: abortSignal
       ? new PromiseCanceller(abortSignal)
       : undefined,
+    completed: false,
   };
   try {
     const {
@@ -319,6 +321,7 @@ function buildResponse(
   exeContext: ExecutionContext,
   data: ObjMap<unknown> | null,
 ): ExecutionResult {
+  exeContext.completed = true;
   exeContext.promiseCanceller?.disconnect();
   const errors = exeContext.errors;
   return errors.length === 0 ? { data } : { errors, data };
@@ -713,6 +716,10 @@ function handleFieldError(
   fieldDetailsList: FieldDetailsList,
   path: Path,
 ): void {
+  if (exeContext.completed) {
+    return;
+  }
+
   const error = locatedError(
     rawError,
     toNodes(fieldDetailsList),
@@ -1275,6 +1282,10 @@ function completeObjectValue(
   path: Path,
   result: unknown,
 ): PromiseOrValue<ObjMap<unknown>> {
+  if (exeContext.completed) {
+    throw new Error('Completed, aborting.');
+  }
+
   // If there is an isTypeOf predicate function, call it with the
   // current result. If isTypeOf returns false, then raise an error rather
   // than continuing execution.