Invoke didEncounterErrors for known errors during pre-parse.

CHANGELOG.md

@@ -6,6 +6,7 @@ The version headers in this history reflect the versions of Apollo Server itself
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.
 
+- `apollo-server-core`: Ensure that plugin's `didEncounterErrors` hooks are invoked for known automated persisted query (APQ) errors. [#3614](https://github.com/apollographql/apollo-server/pull/3614)
 - Move TContext generic from requestDidStart method to ApolloServerPlugin Interface [#3525](https://github.com/apollographql/apollo-server/pull/3525)
 
 ### v2.9.13

packages/apollo-server-core/src/__tests__/runQuery.test.ts

@@ -514,19 +514,19 @@ describe('runQuery', () => {
 
     describe('didEncounterErrors', () => {
       const didEncounterErrors = jest.fn();
+      const plugins: ApolloServerPlugin[] = [
+        {
+          requestDidStart() {
+            return { didEncounterErrors };
+          },
+        },
+      ];
+
       it('called when an error occurs', async () => {
         await runQuery({
           schema,
           queryString: '{ testStringWithParseError: }',
-          plugins: [
-            {
-              requestDidStart() {
-                return {
-                  didEncounterErrors,
-                };
-              },
-            },
-          ],
+          plugins,
           request: new MockReq(),
         });
 
@@ -537,19 +537,32 @@ describe('runQuery', () => {
         );
       });
 
+      it('called when an error occurs in execution', async () => {
+        const response = await runQuery({
+          schema,
+          queryString: '{ testError }',
+          plugins,
+          request: new MockReq(),
+        });
+
+        expect(response).toHaveProperty(
+          'errors.0.message','Secret error message');
+        expect(response).toHaveProperty('data.testError', null);
+
+        expect(didEncounterErrors).toBeCalledWith(
+          expect.objectContaining({
+            errors: expect.arrayContaining([expect.objectContaining({
+              message: 'Secret error message',
+            })]),
+          }),
+        );
+      });
+
       it('not called when an error does not occur', async () => {
         await runQuery({
           schema,
           queryString: '{ testString }',
-          plugins: [
-            {
-              requestDidStart() {
-                return {
-                  didEncounterErrors,
-                };
-              },
-            },
-          ],
+          plugins,
           request: new MockReq(),
         });
 

packages/apollo-server-core/src/requestPipeline.ts

@@ -135,11 +135,10 @@ export async function processGraphQLRequest<TContext>(
     // It looks like we've received a persisted query. Check if we
     // support them.
     if (!config.persistedQueries || !config.persistedQueries.cache) {
-      throw new PersistedQueryNotSupportedError();
+      return await emitErrorAndThrow(new PersistedQueryNotSupportedError());
     } else if (extensions.persistedQuery.version !== 1) {
-      throw new InvalidGraphQLRequestError(
-        'Unsupported persisted query version',
-      );
+      return await emitErrorAndThrow(
+        new InvalidGraphQLRequestError('Unsupported persisted query version'));
     }
 
     // We'll store a reference to the persisted query cache so we can actually
@@ -164,15 +163,14 @@ export async function processGraphQLRequest<TContext>(
       if (query) {
         metrics.persistedQueryHit = true;
       } else {
-        throw new PersistedQueryNotFoundError();
+        return await emitErrorAndThrow(new PersistedQueryNotFoundError());
       }
     } else {
       const computedQueryHash = computeQueryHash(query);
 
       if (queryHash !== computedQueryHash) {
-        throw new InvalidGraphQLRequestError(
-          'provided sha does not match query',
-        );
+        return await emitErrorAndThrow(
+          new InvalidGraphQLRequestError('provided sha does not match query'));
       }
 
       // We won't write to the persisted query cache until later.
@@ -186,7 +184,8 @@ export async function processGraphQLRequest<TContext>(
     // now, but this should be replaced with the new operation ID algorithm.
     queryHash = computeQueryHash(query);
   } else {
-    throw new InvalidGraphQLRequestError('Must provide query string.');
+    return await emitErrorAndThrow(
+      new InvalidGraphQLRequestError('Must provide query string.'));
   }
 
   requestContext.queryHash = queryHash;
@@ -313,6 +312,14 @@ export async function processGraphQLRequest<TContext>(
       // for the time-being this just maintains existing behavior for what
       // happens when `throw`-ing an `HttpQueryError` in `didResolveOperation`.
       if (err instanceof HttpQueryError) {
+        // In order to report this error reliably to the request pipeline, we'll
+        // have to regenerate it with the original error message and stack for
+        // the purposes of the `didEncounterErrors` life-cycle hook (which
+        // expects `GraphQLError`s), but still throw the `HttpQueryError`, so
+        // the appropriate status code is enforced by `runHttpQuery.ts`.
+        const graphqlError = new GraphQLError(err.message);
+        graphqlError.stack = err.stack;
+        await didEncounterErrors([graphqlError]);
         throw err;
       }
       return await sendErrorResponse(err);
@@ -493,6 +500,26 @@ export async function processGraphQLRequest<TContext>(
     return requestContext.response!;
   }
 
+  /**
+   * Report an error via `didEncounterErrors` and then `throw` it.
+   *
+   * Prior to the introduction of this function, some errors were being thrown
+   * within the request pipeline and going directly to handling within
+   * the `runHttpQuery.ts` module, rather than first being reported to the
+   * plugin API's `didEncounterErrors` life-cycle hook (where they are to be
+   * expected!).
+   *
+   * @param error The error to report to the request pipeline plugins prior
+   *              to being thrown.
+   *
+   * @throws
+   *
+   */
+  async function emitErrorAndThrow(error: GraphQLError): Promise<never> {
+    await didEncounterErrors([error]);
+    throw error;
+  }
+
   async function didEncounterErrors(errors: ReadonlyArray<GraphQLError>) {
     requestContext.errors = errors;
     extensionStack.didEncounterErrors(errors);