fix: cancellation should not cause incremental completion error (#4259)

incremental completion errors are indicated if and only if null bubbling has caused a previously sent response to be null we must instead just give a partial response with errors within the incremental array The GraphQL Tools implementation of this feature has different behavior where the next call will throw if the operation is aborted, but we have opted to give partial responses, and so we must adjust.
graphql · Oct 27, 2024 · 83da23a · 83da23a
1 parent cf1c080
commit 83da23a
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 19 deletions.
diff --git a/src/execution/__tests__/abort-signal-test.ts b/src/execution/__tests__/abort-signal-test.ts
@@ -100,8 +100,8 @@ describe('Execute: Cancellation', () => {
       errors: [
         {
           message: 'Aborted',
-          path: ['todo', 'id'],
-          locations: [{ line: 4, column: 11 }],
+          path: ['todo'],
+          locations: [{ line: 3, column: 9 }],
         },
       ],
     });
@@ -149,8 +149,8 @@ describe('Execute: Cancellation', () => {
       errors: [
         {
           message: 'Aborted',
-          path: ['todo', 'author', 'id'],
-          locations: [{ line: 6, column: 13 }],
+          path: ['todo', 'author'],
+          locations: [{ line: 5, column: 11 }],
         },
       ],
     });
@@ -198,8 +198,8 @@ describe('Execute: Cancellation', () => {
       errors: [
         {
           message: 'Aborted',
-          path: ['todo', 'id'],
-          locations: [{ line: 4, column: 11 }],
+          path: ['todo'],
+          locations: [{ line: 3, column: 9 }],
         },
       ],
     });
@@ -257,23 +257,28 @@ describe('Execute: Cancellation', () => {
         hasNext: true,
       },
       {
-        completed: [
+        incremental: [
           {
+            data: {
+              text: 'hello world',
+              author: null,
+            },
             errors: [
               {
                 locations: [
                   {
                     column: 13,
-                    line: 6,
+                    line: 7,
                   },
                 ],
                 message: 'Aborted',
-                path: ['todo', 'text'],
+                path: ['todo', 'author'],
               },
             ],
             id: '0',
           },
         ],
+        completed: [{ id: '0' }],
         hasNext: false,
       },
     ]);

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -729,15 +729,6 @@ function executeFields(
   try {
     for (const [responseName, fieldDetailsList] of groupedFieldSet) {
       const fieldPath = addPath(path, responseName, parentType.name);
-      const abortSignal = exeContext.validatedExecutionArgs.abortSignal;
-      if (abortSignal?.aborted) {
-        throw locatedError(
-          new Error(abortSignal.reason),
-          toNodes(fieldDetailsList),
-          pathToArray(fieldPath),
-        );
-      }
-
       const result = executeField(
         exeContext,
         parentType,
@@ -1737,13 +1728,23 @@ function completeObjectValue(
   incrementalContext: IncrementalContext | undefined,
   deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord> | undefined,
 ): PromiseOrValue<GraphQLWrappedResult<ObjMap<unknown>>> {
+  const validatedExecutionArgs = exeContext.validatedExecutionArgs;
+  const abortSignal = validatedExecutionArgs.abortSignal;
+  if (abortSignal?.aborted) {
+    throw locatedError(
+      new Error(abortSignal.reason),
+      toNodes(fieldDetailsList),
+      pathToArray(path),
+    );
+  }
+
   // 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.
   if (returnType.isTypeOf) {
     const isTypeOf = returnType.isTypeOf(
       result,
-      exeContext.validatedExecutionArgs.contextValue,
+      validatedExecutionArgs.contextValue,
       info,
     );