abort promise and sync for error (#6009)

* abort promise and sync for error

* add changeset

* test

Author: n1ru4l

.changeset/ninety-rats-walk.md

@@ -0,0 +1,5 @@
+---
+"@graphql-tools/executor": patch
+---
+
+correctly raise abort exception for Promise and sync execution

packages/executor/src/execution/__tests__/abort-signal.test.ts

@@ -232,7 +232,7 @@ describe('Abort Signal', () => {
       },
     });
 
-    const result = await normalizedExecutor({
+    const result$ = normalizedExecutor({
       schema,
       document: parse(/* GraphQL */ `
         query {
@@ -241,20 +241,7 @@ describe('Abort Signal', () => {
       `),
       signal: controller.signal,
     });
-    expect(result).toMatchObject({
-      errors: [
-        {
-          message: 'Execution aborted',
-          path: ['counter'],
-          locations: [
-            {
-              line: 3,
-              column: 11,
-            },
-          ],
-        },
-      ],
-    });
+    await expect(result$).rejects.toMatchInlineSnapshot(`DOMException {}`);
     expect(isAborted).toEqual(true);
   });
   it('stops pending stream execution for incremental delivery', async () => {
@@ -319,4 +306,67 @@ describe('Abort Signal', () => {
     await expect(next$).rejects.toMatchInlineSnapshot(`DOMException {}`);
     expect(isReturnInvoked).toEqual(true);
   });
+  it('stops promise execution', async () => {
+    const controller = new AbortController();
+    const d = createDeferred();
+
+    const schema = makeExecutableSchema({
+      typeDefs: /* GraphQL */ `
+        type Query {
+          number: Int!
+        }
+      `,
+      resolvers: {
+        Query: {
+          number: () => d.promise.then(() => 1),
+        },
+      },
+    });
+
+    const result$ = normalizedExecutor({
+      schema,
+      document: parse(/* GraphQL */ `
+        query {
+          number
+        }
+      `),
+      signal: controller.signal,
+    });
+
+    expect(result$).toBeInstanceOf(Promise);
+    controller.abort();
+    await expect(result$).rejects.toMatchInlineSnapshot(`DOMException {}`);
+  });
+  it('does not even try to execute if the signal is already aborted', async () => {
+    const controller = new AbortController();
+    let resolverGotInvoked = false;
+    const schema = makeExecutableSchema({
+      typeDefs: /* GraphQL */ `
+        type Query {
+          number: Int!
+        }
+      `,
+      resolvers: {
+        Query: {
+          number: () => {
+            resolverGotInvoked = true;
+            return 1;
+          },
+        },
+      },
+    });
+    controller.abort();
+    expect(() =>
+      normalizedExecutor({
+        schema,
+        document: parse(/* GraphQL */ `
+          query {
+            number
+          }
+        `),
+        signal: controller.signal,
+      }),
+    ).toThrowErrorMatchingInlineSnapshot(`"This operation was aborted"`);
+    expect(resolverGotInvoked).toBe(false);
+  });
 });

packages/executor/src/execution/execute.ts

@@ -286,6 +286,10 @@ export function execute<TData = any, TVariables = any, TContext = any>(
 function executeImpl<TData = any, TVariables = any, TContext = any>(
   exeContext: ExecutionContext<TVariables, TContext>,
 ): MaybePromise<SingularExecutionResult<TData> | IncrementalExecutionResults<TData>> {
+  if (exeContext.signal?.aborted) {
+    throw exeContext.signal.reason;
+  }
+
   // Return a Promise that will eventually resolve to the data described by
   // The "Response" section of the GraphQL specification.
   //
@@ -297,7 +301,7 @@ function executeImpl<TData = any, TVariables = any, TContext = any>(
   // Errors from sub-fields of a NonNull type may propagate to the top level,
   // at which point we still log the error and null the parent field, which
   // in this case is the entire response.
-  return new ValueOrPromise(() => executeOperation<TData, TVariables, TContext>(exeContext))
+  const result = new ValueOrPromise(() => executeOperation<TData, TVariables, TContext>(exeContext))
     .then(
       data => {
         const initialResult = buildResponse(data, exeContext.errors);
@@ -318,6 +322,30 @@ function executeImpl<TData = any, TVariables = any, TContext = any>(
       },
     )
     .resolve()!;
+
+  if (!exeContext.signal || 'initialResult' in result || 'then' in result === false) {
+    return result;
+  }
+
+  let resolve: () => void;
+  let reject: (reason: any) => void;
+  const abortP = new Promise<never>((_resolve, _reject) => {
+    resolve = _resolve as any;
+    reject = _reject;
+  });
+
+  function abortListener() {
+    reject(exeContext.signal?.reason);
+  }
+
+  exeContext.signal.addEventListener('abort', abortListener);
+
+  result.then(() => {
+    exeContext.signal?.removeEventListener('abort', abortListener);
+    resolve();
+  });
+
+  return Promise.race([abortP, result]);
 }
 
 /**