stop resolvers after execution ends

addresses: #3792
graphql · Oct 29, 2024 · 57552ab · 57552ab
1 parent 99edfea
commit 57552ab
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 24 deletions.
diff --git a/src/execution/__tests__/nonnull-test.ts b/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({

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -164,10 +164,12 @@ export interface ValidatedExecutionArgs {
 export interface ExecutionContext {
   validatedExecutionArgs: ValidatedExecutionArgs;
   aborts: Set<() => void>;
+  completed: boolean;
   cancellableStreams: Set<CancellableStreamRecord> | undefined;
 }
 
 interface IncrementalContext {
+  completed: boolean;
   deferUsageSet?: DeferUsageSet | undefined;
 }
 
@@ -314,6 +316,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
   const exeContext: ExecutionContext = {
     validatedExecutionArgs,
     aborts: new Set(),
+    completed: false,
     cancellableStreams: undefined,
   };
 
@@ -375,15 +378,23 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
 
     if (isPromise(graphqlWrappedResult)) {
       return graphqlWrappedResult.then(
-        (resolved) => buildDataResponse(exeContext, resolved),
-        (error: unknown) => ({
-          data: null,
-          errors: [error as GraphQLError],
-        }),
+        (resolved) => {
+          exeContext.completed = true;
+          return buildDataResponse(exeContext, resolved);
+        },
+        (error: unknown) => {
+          exeContext.completed = true;
+          return {
+            data: null,
+            errors: [error as GraphQLError],
+          };
+        },
       );
     }
+    exeContext.completed = true;
     return buildDataResponse(exeContext, graphqlWrappedResult);
   } catch (error) {
+    exeContext.completed = true;
     return { data: null, errors: [error] };
   }
 }
@@ -831,7 +842,7 @@ function executeField(
     validatedExecutionArgs;
   const fieldName = fieldDetailsList[0].node.name.value;
   const fieldDef = schema.getField(parentType, fieldName);
-  if (!fieldDef) {
+  if (!fieldDef || (incrementalContext ?? exeContext).completed) {
     return;
   }
 
@@ -2277,6 +2288,7 @@ function collectExecutionGroups(
         path,
         groupedFieldSet,
         {
+          completed: false,
           deferUsageSet,
         },
         deferMap,
@@ -2336,6 +2348,7 @@ function executeExecutionGroup(
       deferMap,
     );
   } catch (error) {
+    incrementalContext.completed = true;
     return {
       pendingExecutionGroup,
       path: pathToArray(path),
@@ -2345,16 +2358,26 @@ function executeExecutionGroup(
 
   if (isPromise(result)) {
     return result.then(
-      (resolved) =>
-        buildCompletedExecutionGroup(pendingExecutionGroup, path, resolved),
-      (error: unknown) => ({
-        pendingExecutionGroup,
-        path: pathToArray(path),
-        errors: [error as GraphQLError],
-      }),
+      (resolved) => {
+        incrementalContext.completed = true;
+        return buildCompletedExecutionGroup(
+          pendingExecutionGroup,
+          path,
+          resolved,
+        );
+      },
+      (error: unknown) => {
+        incrementalContext.completed = true;
+        return {
+          pendingExecutionGroup,
+          path: pathToArray(path),
+          errors: [error as GraphQLError],
+        };
+      },
     );
   }
 
+  incrementalContext.completed = true;
   return buildCompletedExecutionGroup(pendingExecutionGroup, path, result);
 }
 
@@ -2403,7 +2426,7 @@ function buildSyncStreamItemQueue(
         initialPath,
         initialItem,
         exeContext,
-        {},
+        { completed: false },
         fieldDetailsList,
         info,
         itemType,
@@ -2434,7 +2457,7 @@ function buildSyncStreamItemQueue(
           itemPath,
           value,
           exeContext,
-          {},
+          { completed: false },
           fieldDetailsList,
           info,
           itemType,
@@ -2526,7 +2549,7 @@ async function getNextAsyncStreamItemResult(
     itemPath,
     iteration.value,
     exeContext,
-    {},
+    { completed: false },
     fieldDetailsList,
     info,
     itemType,
@@ -2573,10 +2596,16 @@ function completeStreamItem(
       incrementalContext,
       new Map(),
     ).then(
-      (resolvedItem) => buildStreamItemResult(resolvedItem),
-      (error: unknown) => ({
-        errors: [error as GraphQLError],
-      }),
+      (resolvedItem) => {
+        incrementalContext.completed = true;
+        return buildStreamItemResult(resolvedItem);
+      },
+      (error: unknown) => {
+        incrementalContext.completed = true;
+        return {
+          errors: [error as GraphQLError],
+        };
+      },
     );
   }
 
@@ -2603,6 +2632,7 @@ function completeStreamItem(
       };
     }
   } catch (error) {
+    incrementalContext.completed = true;
     return {
       errors: [error],
     };
@@ -2618,13 +2648,20 @@ function completeStreamItem(
         ],
       }))
       .then(
-        (resolvedItem) => buildStreamItemResult(resolvedItem),
-        (error: unknown) => ({
-          errors: [error as GraphQLError],
-        }),
+        (resolvedItem) => {
+          incrementalContext.completed = true;
+          return buildStreamItemResult(resolvedItem);
+        },
+        (error: unknown) => {
+          incrementalContext.completed = true;
+          return {
+            errors: [error as GraphQLError],
+          };
+        },
       );
   }
 
+  incrementalContext.completed = true;
   return buildStreamItemResult(result);
 }