fix: bubbling sync errors need not become async (#4661)

Author: yaacovCR

src/execution/Executor.ts

@@ -509,9 +509,7 @@ export class Executor<
     } catch (error) {
       if (containsPromise) {
         // Ensure that any promises returned by other fields are handled, as they may also reject.
-        return promiseForObject(results).finally(() => {
-          throw error;
-        }) as never;
+        promiseForObject(results).catch(() => undefined);
       }
       throw error;
     }
@@ -860,9 +858,7 @@ export class Executor<
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       returnIteratorCatchingErrors(asyncIterator);
       if (containsPromise) {
-        return Promise.all(completedResults).finally(() => {
-          throw error;
-        });
+        Promise.all(completedResults).catch(() => undefined);
       }
       throw error;
     }
@@ -1000,9 +996,7 @@ export class Executor<
       const maybePromises = containsPromise ? completedResults : [];
       maybePromises.push(...collectIteratorPromises(iterator));
       if (maybePromises.length) {
-        return Promise.all(maybePromises).finally(() => {
-          throw error;
-        });
+        Promise.all(maybePromises).catch(() => undefined);
       }
       throw error;
     }

src/execution/__tests__/cancellation-test.ts

@@ -667,6 +667,31 @@ describe('Execute: Cancellation', () => {
     ).to.throw('This operation was aborted');
   });
 
+  it('should stop the execution when aborted before cancellation is wired', async () => {
+    const abortController = new AbortController();
+    const document = parse(`
+      query {
+        blocker
+      }
+    `);
+
+    const resultPromise = execute({
+      document,
+      schema,
+      abortSignal: abortController.signal,
+      rootValue: {
+        blocker: () => {
+          abortController.abort(new Error('Custom abort error'));
+          return new Promise(() => {
+            /* will never resolve */
+          });
+        },
+      },
+    });
+
+    await expectPromise(resultPromise).toRejectWith('Custom abort error');
+  });
+
   it('should stop the execution when aborted prior to return of a subscription resolver', async () => {
     const abortController = new AbortController();
     const document = parse(`

src/execution/__tests__/executor-test.ts

@@ -680,7 +680,6 @@ describe('Execute: Handles basic execution tasks', () => {
 
     const result = execute({ schema, document });
 
-    expect(isAsyncResolverFinished).to.equal(false);
     expectJSON(await result).toDeepEqual({
       data: null,
       errors: [
@@ -692,6 +691,11 @@ describe('Execute: Handles basic execution tasks', () => {
         },
       ],
     });
+
+    expect(isAsyncResolverFinished).to.equal(false);
+    await resolveOnNextTick();
+    await resolveOnNextTick();
+    await resolveOnNextTick();
     expect(isAsyncResolverFinished).to.equal(true);
   });
 

src/execution/__tests__/incremental-test.ts

@@ -167,33 +167,44 @@ describe('Original execute errors on experimental @defer and @stream directives'
       query: new GraphQLObjectType({
         name: 'Query',
         fields: {
-          scalarList: { type: new GraphQLList(GraphQLString) },
-          friendList: {
-            type: new GraphQLList(
-              new GraphQLObjectType({
-                name: 'Friend',
-                fields: {
-                  name: { type: GraphQLString },
+          nested: {
+            type: new GraphQLObjectType({
+              name: 'Nested',
+              fields: {
+                scalarList: { type: new GraphQLList(GraphQLString) },
+                friendList: {
+                  type: new GraphQLList(
+                    new GraphQLObjectType({
+                      name: 'Friend',
+                      fields: {
+                        name: { type: GraphQLString },
+                      },
+                    }),
+                  ),
                 },
-              }),
-            ),
+              },
+            }),
           },
         },
       }),
     });
     const doc = `
       query {
-        scalarList
-        friendList @stream { name }
+        nested {
+          scalarList
+          friendList @stream { name }
+        }
       }
     `;
     await expectPromise(
       execute({
         schema,
         document: parse(doc),
         rootValue: {
-          scalarList: Promise.resolve(['apple', 'banana', 'coconut']),
-          friendList: [{ name: 'Alice' }, { name: 'Bob' }],
+          nested: Promise.resolve({
+            scalarList: Promise.resolve(['apple', 'banana', 'coconut']),
+            friendList: [{ name: 'Alice' }, { name: 'Bob' }],
+          }),
         },
       }),
     ).toRejectWith(

src/execution/legacyIncremental/__tests__/legacy-defer-test.ts

@@ -1710,15 +1710,6 @@ describe('Execute: defer directive (legacy)', () => {
         },
         hasNext: true,
       },
-      {
-        incremental: [
-          {
-            data: { b: { e: { f: 'f' } }, someField: 'someField' },
-            path: ['a'],
-          },
-        ],
-        hasNext: true,
-      },
       {
         incremental: [
           {
@@ -1734,6 +1725,15 @@ describe('Execute: defer directive (legacy)', () => {
             ],
           },
         ],
+        hasNext: true,
+      },
+      {
+        incremental: [
+          {
+            data: { b: { e: { f: 'f' } }, someField: 'someField' },
+            path: ['a'],
+          },
+        ],
         hasNext: false,
       },
     ]);