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

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.
       //