stitching should automatically update path of subschema errors

packages/delegate/src/defaultMergedResolver.ts

@@ -1,6 +1,6 @@
 import { defaultFieldResolver, GraphQLResolveInfo } from 'graphql';
 
-import { getResponseKeyFromInfo, getErrors } from '@graphql-tools/utils';
+import { getResponseKeyFromInfo, getErrors, getDepth } from '@graphql-tools/utils';
 
 import { handleResult } from './results/handleResult';
 import { getSubschema } from './Subschema';
@@ -32,6 +32,7 @@ export function defaultMergedResolver(
 
   const result = parent[responseKey];
   const subschema = getSubschema(parent, responseKey);
+  const depth = getDepth(parent);
 
-  return handleResult(result, errors, subschema, context, info);
+  return handleResult(result, errors, depth, subschema, context, info);
 }

packages/delegate/src/proxiedResult.ts

@@ -1,12 +1,19 @@
-import { GraphQLError } from 'graphql';
-
-import { mergeDeep, ERROR_SYMBOL, relocatedError, setErrors, getErrors } from '@graphql-tools/utils';
+import {
+  mergeDeep,
+  ERROR_SYMBOL,
+  relocatedError,
+  setErrors,
+  getErrors,
+  getDepth,
+  setDepth,
+} from '@graphql-tools/utils';
 
 import { handleNull } from './results/handleNull';
 
 import { FIELD_SUBSCHEMA_MAP_SYMBOL, OBJECT_SUBSCHEMA_SYMBOL } from './symbols';
 import { getSubschema, setObjectSubschema } from './Subschema';
 import { SubschemaConfig } from './types';
+import { GraphQLError } from 'graphql';
 
 export function isProxiedResult(result: any) {
   return result != null ? result[ERROR_SYMBOL] : result;
@@ -18,17 +25,21 @@ export function unwrapResult(parent: any, path: Array<string>): any {
   for (let i = 0; i < pathLength; i++) {
     const responseKey = path[i];
     const errors = getErrors(newParent, responseKey);
+    let depth = getDepth(newParent);
     const subschema = getSubschema(newParent, responseKey);
 
     const object = newParent[responseKey];
     if (object == null) {
       return handleNull(errors);
     }
 
+    depth = depth + 1;
     setErrors(
       object,
-      errors.map(error => relocatedError(error, error.path != null ? error.path.slice(1) : undefined))
+      errors.map(error => relocatedError(error, path.splice(depth, 0, responseKey))),
+      depth
     );
+    setDepth(object, depth);
     setObjectSubschema(object, subschema);
 
     newParent = object;
@@ -52,14 +63,14 @@ export function dehoistResult(parent: any, delimeter = '__gqltf__'): any {
   });
 
   result[ERROR_SYMBOL] = parent[ERROR_SYMBOL].map((error: GraphQLError) => {
-    if (error.path != null) {
-      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));
-    }
-
-    return error;
+    const path = error.relativePath.slice();
+    const pathSegment = path.pop();
+    const expandedPathSegment: Array<string | number> = (pathSegment as string).split(delimeter);
+    return {
+      relativePath: path.concat(expandedPathSegment),
+      // setting path to null will cause issues for errors that bubble up from non nullable fields
+      graphQLError: relocatedError(error.graphQLError, null),
+    };
   });
 
   result[OBJECT_SUBSCHEMA_SYMBOL] = parent[OBJECT_SUBSCHEMA_SYMBOL];
@@ -68,7 +79,7 @@ export function dehoistResult(parent: any, delimeter = '__gqltf__'): any {
 }
 
 export function mergeProxiedResults(target: any, ...sources: any): any {
-  const errors = target[ERROR_SYMBOL].concat(...sources.map((source: any) => source[ERROR_SYMBOL]));
+  const errors = Object.assign(target[ERROR_SYMBOL], ...sources.map((source: any) => source[ERROR_SYMBOL]));
   const fieldSubschemaMap = sources.reduce((acc: Record<any, SubschemaConfig>, source: any) => {
     const subschema = source[OBJECT_SUBSCHEMA_SYMBOL];
     Object.keys(source).forEach(key => {

packages/delegate/src/results/handleList.ts

@@ -19,19 +19,22 @@ import { SubschemaConfig } from '../types';
 export function handleList(
   type: GraphQLList<any>,
   list: Array<any>,
-  errors: ReadonlyArray<GraphQLError>,
+  errors: Array<GraphQLError>,
+  depth: number,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   skipTypeMerging?: boolean
 ) {
-  const childErrors = getErrorsByPathSegment(errors);
+  const newDepth = depth + 1;
+  const childErrors = getErrorsByPathSegment(errors, newDepth);
 
   return list.map((listMember, index) =>
     handleListMember(
       getNullableType(type.ofType),
       listMember,
-      index in childErrors ? childErrors[index] : [],
+      childErrors[index] ?? [],
+      newDepth,
       subschema,
       context,
       info,
@@ -43,7 +46,8 @@ export function handleList(
 function handleListMember(
   type: GraphQLType,
   listMember: any,
-  errors: ReadonlyArray<GraphQLError>,
+  errors: Array<GraphQLError>,
+  depth: number,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
@@ -56,8 +60,8 @@ function handleListMember(
   if (isLeafType(type)) {
     return type.parseValue(listMember);
   } else if (isCompositeType(type)) {
-    return handleObject(type, listMember, errors, subschema, context, info, skipTypeMerging);
+    return handleObject(type, listMember, errors, depth, subschema, context, info, skipTypeMerging);
   } else if (isListType(type)) {
-    return handleList(type, listMember, errors, subschema, context, info, skipTypeMerging);
+    return handleList(type, listMember, errors, depth, subschema, context, info, skipTypeMerging);
   }
 }

packages/delegate/src/results/handleNull.ts

@@ -1,36 +1,13 @@
 import { GraphQLError } from 'graphql';
-
 import AggregateError from 'aggregate-error';
-import { getErrorsByPathSegment, relocatedError } from '@graphql-tools/utils';
 
-export function handleNull(errors: ReadonlyArray<GraphQLError>) {
+export function handleNull(errors: Array<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;
+    if (errors.length > 1) {
+      return new AggregateError(errors);
     }
 
-    const childErrors = getErrorsByPathSegment(errors);
-
-    const result: Array<any> = [];
-    Object.keys(childErrors).forEach(pathSegment => {
-      result.push(handleNull(childErrors[pathSegment]));
-    });
-
-    return result;
+    return errors[0];
   }
 
   return null;

packages/delegate/src/results/handleObject.ts

@@ -8,27 +8,33 @@ import {
   GraphQLResolveInfo,
 } from 'graphql';
 
-import { collectFields, GraphQLExecutionContext, setErrors, slicedError } from '@graphql-tools/utils';
+import {
+  collectFields,
+  GraphQLExecutionContext,
+  setErrors,
+  setDepth,
+  getErrorsByPathSegment,
+} from '@graphql-tools/utils';
 import { setObjectSubschema, isSubschemaConfig } from '../Subschema';
 import { mergeFields } from '../mergeFields';
 import { MergedTypeInfo, SubschemaConfig } from '../types';
 
 export function handleObject(
   type: GraphQLCompositeType,
   object: any,
-  errors: ReadonlyArray<GraphQLError>,
+  errors: Array<GraphQLError>,
+  depth: number,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   skipTypeMerging?: boolean
 ) {
   const stitchingInfo = info?.schema.extensions?.stitchingInfo;
 
-  setErrors(
-    object,
-    errors.map(error => slicedError(error))
-  );
-
+  const newDepth = depth + 1;
+  const newErrorMap = getErrorsByPathSegment(errors, newDepth);
+  setErrors(object, newErrorMap);
+  setDepth(object, newDepth);
   setObjectSubschema(object, subschema);
 
   if (skipTypeMerging || !stitchingInfo) {

packages/delegate/src/results/handleResult.ts

@@ -16,7 +16,8 @@ import { handleList } from './handleList';
 
 export function handleResult(
   result: any,
-  errors: ReadonlyArray<GraphQLError>,
+  errors: Array<GraphQLError>,
+  depth: number,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
@@ -32,8 +33,8 @@ export function handleResult(
   if (isLeafType(type)) {
     return type.parseValue(result);
   } else if (isCompositeType(type)) {
-    return handleObject(type, result, errors, subschema, context, info, skipTypeMerging);
+    return handleObject(type, result, errors, depth, subschema, context, info, skipTypeMerging);
   } else if (isListType(type)) {
-    return handleList(type, result, errors, subschema, context, info, skipTypeMerging);
+    return handleList(type, result, errors, depth, subschema, context, info, skipTypeMerging);
   }
 }

packages/delegate/src/transforms/CheckResultAndHandleErrors.ts

@@ -1,6 +1,6 @@
-import { GraphQLResolveInfo, ExecutionResult, GraphQLOutputType, GraphQLSchema } from 'graphql';
+import { GraphQLResolveInfo, ExecutionResult, GraphQLOutputType, GraphQLSchema, responsePathAsArray } from 'graphql';
 
-import { Transform, getResponseKeyFromInfo } from '@graphql-tools/utils';
+import { Transform, getResponseKeyFromInfo, toGraphQLErrors } from '@graphql-tools/utils';
 import { handleResult } from '../results/handleResult';
 import { SubschemaConfig } from '../types';
 
@@ -50,8 +50,9 @@ export function checkResultAndHandleErrors(
   returnType: GraphQLOutputType = info.returnType,
   skipTypeMerging?: boolean
 ): any {
-  const errors = result.errors != null ? result.errors : [];
+  const sourcePath = info != null ? responsePathAsArray(info.path) : [];
+  const errors = result.errors != null ? toGraphQLErrors(result.errors, sourcePath) : [];
   const data = result.data != null ? result.data[responseKey] : undefined;
 
-  return handleResult(data, errors, subschema, context, info, returnType, skipTypeMerging);
+  return handleResult(data, errors, sourcePath.length - 1, subschema, context, info, returnType, skipTypeMerging);
 }

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);
+    });
   });
 });