diff --git a/.changeset/spotty-days-eat.md b/.changeset/spotty-days-eat.md new file mode 100644 index 00000000000..b033a37649f --- /dev/null +++ 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 index d50df0d6f98..d280d249f9f 100644 --- 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, @@ -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 index c19cdf1cd42..47c121217a2 100644 --- a/packages/server/src/requestPipeline.ts +++ b/packages/server/src/requestPipeline.ts @@ -459,8 +459,6 @@ export async function processGraphQLRequest( ...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( newHTTPGraphQLHead(statusIfExecuteThrows), ); } + + await Promise.all(executionListeners.map((l) => l.executionDidEnd?.())); } await invokeWillSendResponse();