stop resolvers after execution ends (#4263)

depends on #4267

addresses: #3792

Author: yaacovCR

src/execution/__tests__/nonnull-test.ts

@@ -6,6 +6,7 @@ import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
 
 import { invariant } from '../../jsutils/invariant.js';
 import type { PromiseOrValue } from '../../jsutils/PromiseOrValue.js';
+import { promiseWithResolvers } from '../../jsutils/promiseWithResolvers.js';
 
 import { parse } from '../../language/parser.js';
 
@@ -663,6 +664,111 @@ describe('Execute: handles non-nullable types', () => {
       }
       expectJSON(initialErrors).toDeepEqual(result.errors);
     });
+
+    it('suppresses a later error after a parent has been nulled', async () => {
+      const query = `
+        {
+          syncNest {
+            syncNonNull
+            promise
+          }
+        }
+      `;
+
+      const nonNullDeferred = promiseWithResolvers<unknown>();
+      const promiseDeferred = promiseWithResolvers<unknown>();
+
+      const resultPromise = executeQuery(query, {
+        syncNest: {
+          syncNonNull: () => nonNullDeferred.promise,
+          promise: () => promiseDeferred.promise,
+        },
+      });
+
+      nonNullDeferred.reject(syncNonNullError);
+
+      // give first error a chance to propagate
+      await resolveOnNextTick();
+      await resolveOnNextTick();
+      await resolveOnNextTick();
+
+      promiseDeferred.reject(promiseError);
+
+      const result = await resultPromise;
+
+      expectJSON(result).toDeepEqual({
+        data: { syncNest: null },
+        errors: [
+          {
+            message: syncNonNullError.message,
+            path: ['syncNest', 'syncNonNull'],
+            locations: [{ line: 4, column: 13 }],
+          },
+        ],
+      });
+    });
+  });
+
+  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', () => {

src/execution/execute.ts

@@ -184,6 +184,7 @@ export interface ExecutionContext {
   validatedExecutionArgs: ValidatedExecutionArgs;
   collectedErrors: CollectedErrors;
   promiseCanceller: PromiseCanceller | undefined;
+  completed: boolean;
 }
 
 /**
@@ -283,6 +284,7 @@ export function executeQueryOrMutationOrSubscriptionEvent(
     promiseCanceller: abortSignal
       ? new PromiseCanceller(abortSignal)
       : undefined,
+    completed: false,
   };
   try {
     const {
@@ -358,6 +360,7 @@ function buildResponse(
   exeContext: ExecutionContext,
   data: ObjMap<unknown> | null,
 ): ExecutionResult {
+  exeContext.completed = true;
   exeContext.promiseCanceller?.disconnect();
   const errors = exeContext.collectedErrors.errors;
   return errors.length === 0 ? { data } : { errors, data };
@@ -752,6 +755,10 @@ function handleFieldError(
   fieldDetailsList: FieldDetailsList,
   path: Path,
 ): void {
+  if (exeContext.completed) {
+    return;
+  }
+
   const error = locatedError(
     rawError,
     toNodes(fieldDetailsList),
@@ -1306,6 +1313,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.