Ensure executionDidEnd hooks are only called once (#6846)

apollographql · Aug 24, 2022 · 2cab8f7 · 2cab8f7
1 parent 0861ba6
commit 2cab8f7
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 2 deletions.
diff --git a/.changeset/spotty-days-eat.md b/.changeset/spotty-days-eat.md
@@ -0,0 +1,5 @@
+---
+"@apollo/server": patch
+---
+
+Ensure executionDidEnd hooks are only called once (when they throw)
diff --git a/packages/server/src/__tests__/runQuery.test.ts b/packages/server/src/__tests__/runQuery.test.ts
@@ -22,6 +22,7 @@ import {
   GraphQLRequestListenerParsingDidEnd,
   GraphQLRequestListenerValidationDidEnd,
 } from '..';
+import { mockLogger } from './mockLogger';
 
 async function runQuery(
   config: ApolloServerOptions<BaseContext>,
@@ -425,6 +426,44 @@ describe('request pipeline life-cycle hooks', () => {
         expect(executionDidStart).toHaveBeenCalledTimes(1);
         expect(executionDidEnd).toHaveBeenCalledTimes(1);
       });
+
+      it('is only called once if it throws', async () => {
+        const executionDidEnd = jest.fn(() => {
+          throw new Error('boom');
+        });
+
+        const plugins = [
+          {
+            async requestDidStart() {
+              return {
+                async executionDidStart() {
+                  return {
+                    executionDidEnd,
+                  };
+                },
+              };
+            },
+          },
+        ];
+
+        const logger = mockLogger();
+
+        await expect(
+          runQuery(
+            {
+              schema,
+              plugins,
+              logger,
+            },
+            { query: '{ testString }' },
+          ),
+        ).rejects.toThrowError(/Internal server error/);
+
+        expect(executionDidEnd).toHaveBeenCalledTimes(1);
+        expect(logger.error).toHaveBeenCalledWith(
+          'Unexpected error processing request: Error: boom',
+        );
+      });
     });
 
     describe('willResolveField', () => {

diff --git a/packages/server/src/requestPipeline.ts b/packages/server/src/requestPipeline.ts
@@ -459,8 +459,6 @@ export async function processGraphQLRequest<TContext extends BaseContext>(
         ...result,
         errors: resultErrors ? formatErrors(resultErrors) : undefined,
       };
-
-      await Promise.all(executionListeners.map((l) => l.executionDidEnd?.()));
     } catch (executionMaybeError: unknown) {
       const executionError = ensureError(executionMaybeError);
       await Promise.all(
@@ -472,6 +470,8 @@ export async function processGraphQLRequest<TContext extends BaseContext>(
         newHTTPGraphQLHead(statusIfExecuteThrows),
       );
     }
+
+    await Promise.all(executionListeners.map((l) => l.executionDidEnd?.()));
   }
 
   await invokeWillSendResponse();