apollo-engine-reporting: don't fail if errors have non-array 'path' (#…

…3112)

* apollo-engine-reporting: don't fail if errors have non-array 'path'

Specifically, we found that some network errors talking to federated backends
could have a string path.

* Just use `Array.isArray`, which returns `false` when passed `undefined`.

Nit, of course.

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-engine-reporting`: Fix reporting errors which have non-array `path` fields (eg, non-GraphQLError errors). [PR #3112](https://github.com/apollographql/apollo-server/pull/3112)
 - `apollo-engine-reporting`: Add missing `apollo-server-caching` dependency. [PR #3054](https://github.com/apollographql/apollo-server/pull/3054)
 - `apollo-server-hapi`: Revert switch from `accept` and `boom` which took place in v2.8.0. [PR #3089](https://github.com/apollographql/apollo-server/pull/3089)
 - `@apollo/gateway`: Change the `setInterval` timer, which is used to continuously check for updates to a federated graph from the Apollo Graph Manager, to be an `unref`'d timer.  Without this change, the server wouldn't terminate properly once polling had started since the event-loop would continue to have unprocessed events on it. [PR #3105](https://github.com/apollographql/apollo-server/pull/3105)

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

@@ -117,7 +117,9 @@ export class EngineReportingTreeBuilder {
 
     // By default, put errors on the root node.
     let node = this.rootNode;
-    if (path) {
+    // If a non-GraphQLError Error sneaks in here somehow with a non-array
+    // path, don't crash.
+    if (Array.isArray(path)) {
       const specificNode = this.nodes.get(path.join('.'));
       if (specificNode) {
         node = specificNode;

packages/apollo-gateway/src/index.ts

@@ -258,6 +258,11 @@ export class ApolloGateway implements GraphQLService {
     });
   }
 
+  // XXX Nothing guarantees that the only errors thrown or returned in
+  // result.errors are GraphQLErrors, even though other code (eg
+  // apollo-engine-reporting) assumes that. In fact, errors talking to backends
+  // are unlikely to show up as GraphQLErrors. Do we need to use
+  // formatApolloErrors or something?
   public executor = async <TContext>(
     requestContext: WithRequired<
       GraphQLRequestContext<TContext>,

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

@@ -456,6 +456,9 @@ export async function processGraphQLRequest<TContext>(
 
     try {
       if (config.executor) {
+        // XXX Nothing guarantees that the only errors thrown or returned
+        // in result.errors are GraphQLErrors, even though other code
+        // (eg apollo-engine-reporting) assumes that.
         return await config.executor(requestContext);
       } else {
         return await graphql.execute(executionArgs);