fix: Keep special-cased errors (e.g. APQ not found) as unreported.

This fixes the failing tests which correctly surfaced on the last commit.

Previously, prior to the new plugin API, the Apollo Engine Reporting
mechanism was implemented using `graphql-extensions`, the API for which
didn't invoke `requestDidStart` until _after_ APQ had been negotiated.

The new plugin API starts its `requestDidStart` _before_ APQ validation and
various other assertions which weren't included in the `requestDidStart`
life-cycle, even if they perhaps should be in terms of error reporting.

The new plugin API is able to properly capture such errors within its
`didEncounterErrors` lifecycle hook (thanks to
#3614, which
intentionally captures these failures so plugin authors can accurately react
to them), however, for behavioral consistency reasons, we will still
special-case those errors and maintain the legacy behavior to avoid a
breaking change.  We can reconsider this in a future version of Apollo
Engine Reporting (AS3, perhaps!).

Ref: #3614
Ref: #3627
Ref: #3638

packages/apollo-engine-reporting/src/extension.ts

@@ -3,6 +3,7 @@ import {
   WithRequired,
   Logger,
   SchemaHash,
+  InvalidGraphQLRequestError,
 } from 'apollo-server-types';
 import { Request, Headers } from 'apollo-server-env';
 import {
@@ -23,6 +24,10 @@ import {
 } from './agent';
 import { EngineReportingTreeBuilder } from './treeBuilder';
 import { ApolloServerPlugin } from "apollo-server-plugin-base";
+import {
+  PersistedQueryNotFoundError,
+  PersistedQueryNotSupportedError,
+} from 'apollo-server-errors';
 
 type Mutable<T> = { -readonly [P in keyof T]: T[P] };
 
@@ -373,6 +378,12 @@ export const plugin = <TContext>(
         },
 
         didEncounterErrors({ errors }) {
+          // We don't report some special-cased errors to Graph Manager.
+          // See the definition of this function for the reasons.
+          if (allUnreportableSpecialCasedErrors(errors)) {
+            return;
+          }
+
           ensurePreflight();
           treeBuilder.didEncounterErrors(errors);
           didEnd();
@@ -382,6 +393,39 @@ export const plugin = <TContext>(
   };
 };
 
+/**
+ * Previously, prior to the new plugin API, the Apollo Engine Reporting
+ * mechanism was implemented using `graphql-extensions`, the API for which
+ * didn't invoke `requestDidStart` until _after_ APQ had been negotiated.
+ *
+ * The new plugin API starts its `requestDidStart` _before_ APQ validation and
+ * various other assertions which weren't included in the `requestDidStart`
+ * life-cycle, even if they perhaps should be in terms of error reporting.
+ *
+ * The new plugin API is able to properly capture such errors within its
+ * `didEncounterErrors` lifecycle hook, however, for behavioral consistency
+ * reasons, we will still special-case those errors and maintain the legacy
+ * behavior to avoid a breaking change.  We can reconsider this in a future
+ * version of Apollo Engine Reporting (AS3, perhaps!).
+ *
+ * @param errors A list of errors to scan for special-cased instances.
+ */
+function allUnreportableSpecialCasedErrors(
+  errors: readonly GraphQLError[],
+): boolean {
+  return errors.every(err => {
+    if (
+      err instanceof PersistedQueryNotFoundError ||
+      err instanceof PersistedQueryNotSupportedError ||
+      err instanceof InvalidGraphQLRequestError
+    ) {
+      return true;
+    }
+
+    return false;
+  });
+}
+
 // Helpers for producing traces.
 
 function defaultGenerateClientInfo({ request }: GraphQLRequestContext) {

packages/apollo-engine-reporting/src/treeBuilder.ts

@@ -1,10 +1,6 @@
 import { GraphQLError, GraphQLResolveInfo, ResponsePath } from 'graphql';
 import { Trace, google } from 'apollo-engine-reporting-protobuf';
-import {
-  PersistedQueryNotFoundError,
-  PersistedQueryNotSupportedError,
-} from 'apollo-server-errors';
-import { InvalidGraphQLRequestError, Logger } from 'apollo-server-types';
+import { Logger } from 'apollo-server-types';
 
 function internalError(message: string) {
   return new Error(`[internal apollo-server error] ${message}`);
@@ -80,14 +76,6 @@ export class EngineReportingTreeBuilder {
 
   public didEncounterErrors(errors: readonly GraphQLError[]) {
     errors.forEach(err => {
-      if (
-        err instanceof PersistedQueryNotFoundError ||
-        err instanceof PersistedQueryNotSupportedError ||
-        err instanceof InvalidGraphQLRequestError
-      ) {
-        return;
-      }
-
       // This is an error from a federated service. We will already be reporting
       // it in the nested Trace in the query plan.
       //