Revert "Gateway: pass through errors encountered when running final e…

…xecute (#159)"

This reverts commit 9045f55, which was from
#159.

See the comment in #981
which explains the justification as well, but briefly:

While that PR was an improvement in principle, it:

- has a bug that was first captured in
  #159 (comment)
To quote that here:

  > I think there's a bug here. Let's say we have a query `{ x }` where `x`
is a non-nullable field, and the federated execution has `x` throw an error.
This turns all of `data` into `null` (not `{ x: null }`). But then this
re-execution adds another error saying that the non-null field is null.
  >
  > Take a look at
https://codesandbox.io/s/angry-raman-uuzuf?file=/src/index.js for an example
of what's going on here.

- Sometimes — unexpectedly to current users, at least — breaks client
  expectations.

As of now, the pain points seem to be outweighing the gains.  We'll need to
revert this and revisit this when time allows with a slightly different
approach and #981 tracks
the need to do so.

Ref: #159
Ref: apollographql/apollo-server#5550
Ref: #974
Ref: apollographql/apollo-server#4523

gateway-js/src/__tests__/executeQueryPlan.test.ts

@@ -1322,6 +1322,23 @@ describe('executeQueryPlan', () => {
       `);
     });
 
+    // THIS TEST SHOULD BE MODIFIED AFTER THE ISSUE OUTLINED IN
+    // https://github.com/apollographql/federation/issues/981 HAS BEEN RESOLVED.
+    // IT IS BEING LEFT HERE AS A TEST THAT WILL INTENTIONALLY FAIL WHEN
+    // IT IS RESOLVED IF IT'S NOT ADDRESSED.
+    //
+    // This test became relevant after a combination of two things:
+    //   1. when the gateway started surfacing errors from subgraphs happened in
+    //      https://github.com/apollographql/federation/pull/159
+    //   2. the idea of field redaction became necessary after
+    //      https://github.com/apollographql/federation/pull/893,
+    //      which introduced the notion of inaccessible fields.
+    //      The redaction started in
+    //      https://github.com/apollographql/federation/issues/974, which added
+    //      the following test.
+    //
+    // However, the error surfacing (first, above) needed to be reverted, thus
+    // de-necessitating this redaction logic which is no longer tested.
     it(`doesn't leak @inaccessible typenames in error messages`, async () => {
       const operationString = `#graphql
         query {
@@ -1353,11 +1370,13 @@ describe('executeQueryPlan', () => {
       );
 
       expect(response.data?.vehicle).toEqual(null);
-      expect(response.errors).toMatchInlineSnapshot(`
-        Array [
-          [GraphQLError: Abstract type "Vehicle" was resolve to a type [inaccessible type] that does not exist inside schema.],
-        ]
-      `);
+      expect(response.errors).toBeUndefined();
+      // SEE COMMENT ABOVE THIS TEST.  SHOULD BE RE-ENABLED AFTER #981 IS FIXED!
+      // expect(response.errors).toMatchInlineSnapshot(`
+      //   Array [
+      //     [GraphQLError: Abstract type "Vehicle" was resolve to a type [inaccessible type] that does not exist inside schema.],
+      //   ]
+      // `);
     });
   });
 });

gateway-js/src/executeQueryPlan.ts

@@ -31,7 +31,6 @@ import { deepMerge } from './utilities/deepMerge';
 import { isNotNullOrUndefined } from './utilities/array';
 import { SpanStatusCode } from "@opentelemetry/api";
 import { OpenTelemetrySpanNames, tracer } from "./utilities/opentelemetry";
-import { cleanErrorOfInaccessibleNames } from './utilities/cleanErrorOfInaccessibleNames';
 
 export type ServiceMap = {
   [serviceName: string]: GraphQLDataSource;
@@ -53,6 +52,9 @@ export async function executeQueryPlan<TContext>(
   requestContext: GraphQLRequestContext<TContext>,
   operationContext: OperationContext,
 ): Promise<GraphQLExecutionResult> {
+
+  const logger = requestContext.logger || console;
+
   return tracer.startActiveSpan(OpenTelemetrySpanNames.EXECUTE, async span => {
     try {
       const errors: GraphQLError[] = [];
@@ -92,7 +94,7 @@ export async function executeQueryPlan<TContext>(
         // It is also used to allow execution of introspection queries though.
         try {
           const schema = toAPISchema(operationContext.schema);
-          const executionResult = await execute({
+          ({ data } = await execute({
             schema,
             document: {
               kind: Kind.DOCUMENT,
@@ -105,17 +107,38 @@ export async function executeQueryPlan<TContext>(
             variableValues: requestContext.request.variables,
             // See also `wrapSchemaWithAliasResolver` in `gateway-js/src/index.ts`.
             fieldResolver: defaultFieldResolverWithAliasSupport,
-          });
-          data = executionResult.data;
-          if (executionResult.errors?.length) {
-            const cleanedErrors = executionResult.errors.map((error) =>
-              cleanErrorOfInaccessibleNames(schema, error),
-            );
-            errors.push(...cleanedErrors);
-          }
+          }));
         } catch (error) {
           span.setStatus({ code:SpanStatusCode.ERROR });
-          return { errors: [error] };
+          if (error instanceof GraphQLError) {
+            return { errors: [error] };
+          } else if (error instanceof Error) {
+            return {
+              errors: [
+                new GraphQLError(
+                  error.message,
+                  undefined,
+                  undefined,
+                  undefined,
+                  undefined,
+                  error as Error,
+                )
+              ]
+            };
+          } else {
+            // The above cases should cover the known cases, but if we received
+            // something else in the `catch` — like an object or something, we
+            // may not want to merely return this to the client.
+            logger.error(
+              "Unexpected error during query plan execution: " + error);
+            return {
+              errors: [
+                new GraphQLError(
+                  "Unexpected error during query plan execution",
+                )
+              ]
+            };
+          }
         }
         finally {
           span.end()