fix(errors): preserve merged paths

This is not the optimal way to address #1047 but is the most backwards
compatible.

Author: yaacovCR

packages/delegate/src/createMergedResolver.ts

@@ -1,6 +1,13 @@
 import { GraphQLError } from 'graphql';
 
-import { IFieldResolver, getErrors, setErrors, relocatedError, ERROR_SYMBOL } from '@graphql-tools/utils';
+import {
+  IFieldResolver,
+  getErrors,
+  setErrors,
+  relocatedError,
+  ERROR_SYMBOL,
+  extendedError,
+} from '@graphql-tools/utils';
 
 import { OBJECT_SUBSCHEMA_SYMBOL } from './symbols';
 
@@ -54,7 +61,15 @@ function dehoistResult(parent: any, delimeter = '__gqltf__'): any {
       const path = error.path.slice();
       const pathSegment = path.shift();
       const expandedPathSegment: Array<string | number> = (pathSegment as string).split(delimeter);
-      return relocatedError(error, expandedPathSegment.concat(path));
+      const newPath = expandedPathSegment.concat(path);
+      const graphQLToolsMergedPath = error.extensions.graphQLToolsMergedPath;
+      let newGraphQLToolsMergedPath = graphQLToolsMergedPath.slice();
+      newGraphQLToolsMergedPath.pop();
+      newGraphQLToolsMergedPath = newGraphQLToolsMergedPath.concat(newPath);
+      return extendedError(relocatedError(error, newPath), {
+        ...error.extensions,
+        graphQLToolsMergedPath: newGraphQLToolsMergedPath,
+      });
     }
 
     return error;

packages/delegate/src/results/handleNull.ts

@@ -1,36 +1,28 @@
 import { GraphQLError } from 'graphql';
 
 import AggregateError from '@ardatan/aggregate-error';
-import { getErrorsByPathSegment, relocatedError } from '@graphql-tools/utils';
+
+import { relocatedError, unextendedError } from '@graphql-tools/utils';
 
 export function handleNull(errors: ReadonlyArray<GraphQLError>) {
   if (errors.length) {
-    if (errors.some(error => !error.path || error.path.length < 2)) {
-      if (errors.length > 1) {
-        const combinedError = new AggregateError(errors);
-        return combinedError;
-      }
-      const error = errors[0];
-      return error.originalError || relocatedError(error, null);
-    } else if (errors.some(error => typeof error.path[1] === 'string')) {
-      const childErrors = getErrorsByPathSegment(errors);
-
-      const result = {};
-      Object.keys(childErrors).forEach(pathSegment => {
-        result[pathSegment] = handleNull(childErrors[pathSegment]);
-      });
-
-      return result;
+    const graphQLToolsMergedPath = errors[0].extensions.graphQLToolsMergedPath;
+    const unannotatedErrors = errors.map(error => unextendedError(error, 'graphQLToolsMergedPath'));
+
+    if (unannotatedErrors.length > 1) {
+      const combinedError = new AggregateError(unannotatedErrors);
+      return new GraphQLError(
+        combinedError.message,
+        undefined,
+        undefined,
+        undefined,
+        graphQLToolsMergedPath,
+        combinedError
+      );
     }
 
-    const childErrors = getErrorsByPathSegment(errors);
-
-    const result: Array<any> = [];
-    Object.keys(childErrors).forEach(pathSegment => {
-      result.push(handleNull(childErrors[pathSegment]));
-    });
-
-    return result;
+    const error = unannotatedErrors[0];
+    return relocatedError(error, graphQLToolsMergedPath);
   }
 
   return null;

packages/delegate/src/results/handleResult.ts

@@ -6,13 +6,15 @@ import {
   isListType,
   GraphQLError,
   GraphQLSchema,
+  responsePathAsArray,
 } from 'graphql';
 
 import { SubschemaConfig } from '../types';
 
 import { handleNull } from './handleNull';
 import { handleObject } from './handleObject';
 import { handleList } from './handleList';
+import { extendedError } from '@graphql-tools/utils';
 
 export function handleResult(
   result: any,
@@ -23,17 +25,32 @@ export function handleResult(
   returnType = info.returnType,
   skipTypeMerging?: boolean
 ): any {
+  const annotatedErrors = errors.map(error => {
+    if (error.extensions?.graphQLToolsMergedPath == null) {
+      return extendedError(error, {
+        ...error.extensions,
+        graphQLToolsMergedPath:
+          info == null
+            ? error.path
+            : error.path != null
+            ? [...responsePathAsArray(info.path), ...error.path.slice(1)]
+            : responsePathAsArray(info.path),
+      });
+    }
+    return error;
+  });
+
   const type = getNullableType(returnType);
 
   if (result == null) {
-    return handleNull(errors);
+    return handleNull(annotatedErrors);
   }
 
   if (isLeafType(type)) {
     return type.parseValue(result);
   } else if (isCompositeType(type)) {
-    return handleObject(type, result, errors, subschema, context, info, skipTypeMerging);
+    return handleObject(type, result, annotatedErrors, subschema, context, info, skipTypeMerging);
   } else if (isListType(type)) {
-    return handleList(type, result, errors, subschema, context, info, skipTypeMerging);
+    return handleList(type, result, annotatedErrors, subschema, context, info, skipTypeMerging);
   }
 }

packages/delegate/src/results/mergeFields.ts

@@ -170,21 +170,21 @@ export function mergeFields(
   }
 
   let containsPromises = false;
-  const maybePromises: Promise<any> | any = [];
+  const resultMap: Map<Promise<any> | any, SelectionSetNode> = new Map();
   delegationMap.forEach((selectionSet: SelectionSetNode, s: SubschemaConfig) => {
     const maybePromise = s.merge[typeName].resolve(object, context, info, s, selectionSet);
-    maybePromises.push(maybePromise);
-    if (!containsPromises && isPromise(maybePromise)) {
+    resultMap.set(maybePromise, selectionSet);
+    if (isPromise(maybePromise)) {
       containsPromises = true;
     }
   });
 
   return containsPromises
-    ? Promise.all(maybePromises).then(results =>
+    ? Promise.all(resultMap.keys()).then(results =>
         mergeFields(
           mergedTypeInfo,
           typeName,
-          mergeProxiedResults(object, ...results),
+          mergeProxiedResults(info, object, results, Array.from(resultMap.values())),
           unproxiableFieldNodes,
           combineSubschemas(sourceSubschemaOrSourceSubschemas, proxiableSubschemas),
           nonProxiableSubschemas,
@@ -195,7 +195,7 @@ export function mergeFields(
     : mergeFields(
         mergedTypeInfo,
         typeName,
-        mergeProxiedResults(object, ...maybePromises),
+        mergeProxiedResults(info, object, Array.from(resultMap.keys()), Array.from(resultMap.values())),
         unproxiableFieldNodes,
         combineSubschemas(sourceSubschemaOrSourceSubschemas, proxiableSubschemas),
         nonProxiableSubschemas,

packages/delegate/src/results/mergeProxiedResults.ts

@@ -1,18 +1,51 @@
-import { mergeDeep, ERROR_SYMBOL } from '@graphql-tools/utils';
+import { GraphQLError, GraphQLResolveInfo, responsePathAsArray, SelectionSetNode, GraphQLObjectType } from 'graphql';
+
+import {
+  mergeDeep,
+  ERROR_SYMBOL,
+  extendedError,
+  collectFields,
+  GraphQLExecutionContext,
+  relocatedError,
+} from '@graphql-tools/utils';
 
 import { SubschemaConfig } from '../types';
 import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL } from '../symbols';
 
-export function mergeProxiedResults(target: any, ...sources: Array<any>): any {
+export function mergeProxiedResults(
+  info: GraphQLResolveInfo,
+  target: any,
+  sources: Array<any>,
+  selectionSets: Array<SelectionSetNode>
+): any {
   const results: Array<any> = [];
-  const errors: Array<Error> = [];
+  let errors: Array<GraphQLError> = [];
+
+  const path = responsePathAsArray(info.path);
 
-  sources.forEach(source => {
-    if (source instanceof Error) {
-      errors.push(source);
+  sources.forEach((source, index) => {
+    if (source instanceof GraphQLError) {
+      const selectionSet = selectionSets[index];
+      const fieldNodes = collectFields(
+        {
+          schema: info.schema,
+          variableValues: {},
+          fragments: {},
+        } as GraphQLExecutionContext,
+        info.schema.getType(target.__typename) as GraphQLObjectType,
+        selectionSet,
+        Object.create(null),
+        Object.create(null)
+      );
+      const nullResult = {};
+      Object.keys(fieldNodes).forEach(responseKey => {
+        errors.push(relocatedError(source, [responseKey]));
+        nullResult[responseKey] = null;
+      });
+      results.push(nullResult);
     } else {
+      errors = errors.concat(source[ERROR_SYMBOL]);
       results.push(source);
-      errors.push(source[ERROR_SYMBOL]);
     }
   });
 
@@ -29,7 +62,14 @@ export function mergeProxiedResults(target: any, ...sources: Array<any>): any {
     ? Object.assign({}, target[FIELD_SUBSCHEMA_MAP_SYMBOL], fieldSubschemaMap)
     : fieldSubschemaMap;
 
-  result[ERROR_SYMBOL] = target[ERROR_SYMBOL].concat(...errors);
+  const annotatedErrors = errors.map(error => {
+    return extendedError(error, {
+      ...error.extensions,
+      graphQLToolsMergedPath: error.path != null ? [...path, ...error.path] : responsePathAsArray(info.path),
+    });
+  });
+
+  result[ERROR_SYMBOL] = target[ERROR_SYMBOL].concat(annotatedErrors);
 
   return result;
 }

packages/stitch/tests/errors.test.ts

@@ -38,6 +38,7 @@ describe('passes along errors for missing fields on list', () => {
     const originalResult = await graphql(schema, query);
     const stitchedResult = await graphql(stitchedSchema, query);
     expect(stitchedResult).toEqual(originalResult);
+    expect(stitchedResult.errors[0].path).toEqual(originalResult.errors[0].path);
   });
 
   test('even if nullable', async () => {
@@ -72,6 +73,7 @@ describe('passes along errors for missing fields on list', () => {
     const originalResult = await graphql(schema, query);
     const stitchedResult = await graphql(stitchedSchema, query);
     expect(stitchedResult).toEqual(originalResult);
+    expect(stitchedResult.errors[0].path).toEqual(originalResult.errors[0].path);
   });
 });
 
@@ -108,6 +110,7 @@ describe('passes along errors when list field errors', () => {
     const originalResult = await graphql(schema, query);
     const stitchedResult = await graphql(stitchedSchema, query);
     expect(stitchedResult).toEqual(originalResult);
+    expect(stitchedResult.errors[0].path).toEqual(originalResult.errors[0].path);
   });
 
   test('even if nullable', async () => {
@@ -142,6 +145,38 @@ describe('passes along errors when list field errors', () => {
     const originalResult = await graphql(schema, query);
     const stitchedResult = await graphql(stitchedSchema, query);
     expect(stitchedResult).toEqual(originalResult);
+    expect(stitchedResult.errors[0].path).toEqual(originalResult.errors[0].path);
+  });
+
+  describe('passes along correct error when there are two non-null fields', () => {
+    test('should work', async () => {
+      const schema = makeExecutableSchema({
+        typeDefs: `
+          type Query {
+            getBoth: Both
+          }
+          type Both {
+            mandatoryField1: String!
+            mandatoryField2: String!
+          }
+        `,
+        resolvers: {
+          Query: {
+            getBoth: () => ({ mandatoryField1: 'test' }),
+          },
+        },
+      });
+
+      const stitchedSchema = stitchSchemas({
+        subschemas: [schema],
+      });
+
+      const query = '{ getBoth { mandatoryField1 mandatoryField2 } }';
+      const originalResult = await graphql(schema, query);
+      const stitchedResult = await graphql(stitchedSchema, query);
+      expect(stitchedResult).toEqual(originalResult);
+      expect(stitchedResult.errors[0].path).toEqual(originalResult.errors[0].path);
+    });
   });
 });
 

packages/utils/src/errors.ts

@@ -14,6 +14,63 @@ export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray
   );
 }
 
+export function extendedError(originalError: GraphQLError, extensions: Record<string, any>): GraphQLError {
+  return new GraphQLError(
+    originalError.message,
+    originalError.nodes,
+    originalError.source,
+    originalError.positions,
+    originalError.path,
+    originalError.originalError,
+    extensions
+  );
+}
+
+export function unextendedError(originalError: GraphQLError, extensionKey: string): GraphQLError {
+  const originalExtensions = originalError.extensions;
+
+  if (originalExtensions == null) {
+    return originalError;
+  }
+
+  const originalExtensionKeys = Object.keys(originalExtensions);
+
+  if (!originalExtensionKeys.length) {
+    return originalError;
+  }
+
+  const newExtensions = {};
+  let extensionsPresent = false;
+  originalExtensionKeys.forEach(key => {
+    if (key !== extensionKey) {
+      newExtensions[key] = originalExtensions[key];
+      extensionsPresent = true;
+    }
+  });
+
+  if (!extensionsPresent) {
+    return new GraphQLError(
+      originalError.message,
+      originalError.nodes,
+      originalError.source,
+      originalError.positions,
+      originalError.path,
+      originalError.originalError,
+      undefined
+    );
+  }
+
+  return new GraphQLError(
+    originalError.message,
+    originalError.nodes,
+    originalError.source,
+    originalError.positions,
+    originalError.path,
+    originalError.originalError,
+    newExtensions
+  );
+}
+
 export function slicedError(originalError: GraphQLError) {
   return relocatedError(originalError, originalError.path != null ? originalError.path.slice(1) : undefined);
 }