From 9817b005e6c7e3641836c860b6bd545ec97497a3 Mon Sep 17 00:00:00 2001
From: Chris Karcher <chris@chriskarcher.net>
Date: Thu, 18 Aug 2022 13:28:37 -0500
Subject: [PATCH 1/2] Fix crash in node when mixing sync/async resolvers
 (backport of #3706)

---
 src/execution/__tests__/executor-test.ts | 50 ++++++++++++++++++++++++
 src/execution/execute.ts                 | 36 +++++++++++------
 2 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts
index 116334aded..9520b06748 100644
--- a/src/execution/__tests__/executor-test.ts
+++ b/src/execution/__tests__/executor-test.ts
@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { expectJSON } from '../../__testUtils__/expectJSON';
+import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick';
 
 import { inspect } from '../../jsutils/inspect';
 import { invariant } from '../../jsutils/invariant';
@@ -625,6 +626,55 @@ describe('Execute: Handles basic execution tasks', () => {
     });
   });
 
+  it('handles sync errors combined with rejections', async () => {
+    let isAsyncResolverCalled = false;
+
+    const schema = new GraphQLSchema({
+      query: new GraphQLObjectType({
+        name: 'Query',
+        fields: {
+          syncNullError: {
+            type: new GraphQLNonNull(GraphQLString),
+            resolve: () => null,
+          },
+          asyncNullError: {
+            type: new GraphQLNonNull(GraphQLString),
+            async resolve() {
+              await resolveOnNextTick();
+              await resolveOnNextTick();
+              await resolveOnNextTick();
+              isAsyncResolverCalled = true;
+              return Promise.resolve(null);
+            },
+          },
+        },
+      }),
+    });
+
+    // Order is important here, as the promise has to be created before the synchronous error is thrown
+    const document = parse(`
+      {
+        asyncNullError
+        syncNullError
+      }
+    `);
+
+    const result = await execute({ schema, document });
+
+    expect(isAsyncResolverCalled).to.equal(true);
+    expectJSON(result).toDeepEqual({
+      data: null,
+      errors: [
+        {
+          message:
+            'Cannot return null for non-nullable field Query.syncNullError.',
+          locations: [{ line: 4, column: 9 }],
+          path: ['syncNullError'],
+        },
+      ],
+    });
+  });
+
   it('Full response path is included for non-nullable fields', () => {
     const A: GraphQLObjectType = new GraphQLObjectType({
       name: 'A',
diff --git a/src/execution/execute.ts b/src/execution/execute.ts
index 4b8cf3a6f7..55c22ea9de 100644
--- a/src/execution/execute.ts
+++ b/src/execution/execute.ts
@@ -445,22 +445,32 @@ function executeFields(
   const results = Object.create(null);
   let containsPromise = false;
 
-  for (const [responseName, fieldNodes] of fields.entries()) {
-    const fieldPath = addPath(path, responseName, parentType.name);
-    const result = executeField(
-      exeContext,
-      parentType,
-      sourceValue,
-      fieldNodes,
-      fieldPath,
-    );
+  try {
+    for (const [responseName, fieldNodes] of fields.entries()) {
+      const fieldPath = addPath(path, responseName, parentType.name);
+      const result = executeField(
+        exeContext,
+        parentType,
+        sourceValue,
+        fieldNodes,
+        fieldPath,
+      );
 
-    if (result !== undefined) {
-      results[responseName] = result;
-      if (isPromise(result)) {
-        containsPromise = true;
+      if (result !== undefined) {
+        results[responseName] = result;
+        if (isPromise(result)) {
+          containsPromise = true;
+        }
       }
     }
+  } 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;
+      });
+    }
+    throw error;
   }
 
   // If there are no promises, we can just return the object

From 40909c8ddf3a14930850131d57d437eaf0157d1c Mon Sep 17 00:00:00 2001
From: Ivan Goncharov <ivan.goncharov.ua@gmail.com>
Date: Wed, 5 Oct 2022 18:03:46 +0300
Subject: [PATCH 2/2] review changes

---
 src/execution/__tests__/executor-test.ts | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts
index 9520b06748..c758d3e426 100644
--- a/src/execution/__tests__/executor-test.ts
+++ b/src/execution/__tests__/executor-test.ts
@@ -627,7 +627,7 @@ describe('Execute: Handles basic execution tasks', () => {
   });
 
   it('handles sync errors combined with rejections', async () => {
-    let isAsyncResolverCalled = false;
+    let isAsyncResolverFinished = false;
 
     const schema = new GraphQLSchema({
       query: new GraphQLObjectType({
@@ -643,8 +643,8 @@ describe('Execute: Handles basic execution tasks', () => {
               await resolveOnNextTick();
               await resolveOnNextTick();
               await resolveOnNextTick();
-              isAsyncResolverCalled = true;
-              return Promise.resolve(null);
+              isAsyncResolverFinished = true;
+              return null;
             },
           },
         },
@@ -659,10 +659,10 @@ describe('Execute: Handles basic execution tasks', () => {
       }
     `);
 
-    const result = await execute({ schema, document });
+    const result = execute({ schema, document });
 
-    expect(isAsyncResolverCalled).to.equal(true);
-    expectJSON(result).toDeepEqual({
+    expect(isAsyncResolverFinished).to.equal(false);
+    expectJSON(await result).toDeepEqual({
       data: null,
       errors: [
         {
@@ -673,6 +673,7 @@ describe('Execute: Handles basic execution tasks', () => {
         },
       ],
     });
+    expect(isAsyncResolverFinished).to.equal(true);
   });
 
   it('Full response path is included for non-nullable fields', () => {