fix(batch-delegate): proxy batched errors (#2288)

gmac · web-flow · commit 29ead57c195a · 2020-11-29T22:48:09.000+02:00
Previously, batch-delegate did not reset the path on errors, so an array index would spuriously end up in the path. The new onLocatedError option to delegateToSchema allows batchDelegateToSchema to remove the array index after the array index is used to properly merge the error into the returned object. Previously, any error returned by the batch loader would result in throwing of a new error => now errors returned by the batch loader are caught and returned as is (cf. https://github.com/graphql/graphql-js/blob/cd273ad136d615b3f2f4c830bd8891c7c5590c30/src/execution/execute.js#L693)
diff --git a/.changeset/seven-dryers-vanish.md b/.changeset/seven-dryers-vanish.md
@@ -0,0 +1,6 @@
+---
+'@graphql-tools/batch-delegate': patch
+'@graphql-tools/delegate': patch
+---
+
+fix(batch-delegate): proxy batched errors
diff --git a/packages/batch-delegate/src/getLoader.ts b/packages/batch-delegate/src/getLoader.ts
@@ -3,6 +3,7 @@ import { getNamedType, GraphQLOutputType, GraphQLList, GraphQLSchema } from 'gra
 import DataLoader from 'dataloader';
 
 import { delegateToSchema, SubschemaConfig } from '@graphql-tools/delegate';
+import { relocatedError } from '@graphql-tools/utils';
 
 import { BatchDelegateOptions, DataLoaderCache } from './types';
 
@@ -15,10 +16,16 @@ function createBatchFn<K = any>(options: BatchDelegateOptions) {
   return async (keys: ReadonlyArray<K>) => {
     const results = await delegateToSchema({
       returnType: new GraphQLList(getNamedType(options.info.returnType) as GraphQLOutputType),
+      onLocatedError: originalError =>
+        relocatedError(originalError, originalError.path.slice(0, 0).concat(originalError.path.slice(2))),
       args: argsFromKeys(keys),
       ...(lazyOptionsFn == null ? options : lazyOptionsFn(options)),
     });
 
+    if (results instanceof Error) {
+      return keys.map(() => results);
+    }
+
     const values = valuesFromResults == null ? results : valuesFromResults(results, keys);
 
     return Array.isArray(values) ? values : keys.map(() => values);
diff --git a/packages/delegate/src/delegateToSchema.ts b/packages/delegate/src/delegateToSchema.ts
@@ -92,6 +92,7 @@ export function delegateRequest({
   fieldName,
   args,
   returnType,
+  onLocatedError,
   context,
   transforms = [],
   transformedSchema,
@@ -134,6 +135,7 @@ export function delegateRequest({
     info,
     returnType:
       returnType ?? info?.returnType ?? getDelegationReturnType(targetSchema, targetOperation, targetFieldName),
+    onLocatedError,
     transforms: allTransforms,
     transformedSchema: transformedSchema ?? (subschemaConfig as Subschema)?.transformedSchema ?? targetSchema,
     skipTypeMerging,
diff --git a/packages/delegate/src/mergeFields.ts b/packages/delegate/src/mergeFields.ts
@@ -180,11 +180,12 @@ export function mergeFields(
   const resultMap: Map<Promise<any> | any, SelectionSetNode> = new Map();
   delegationMap.forEach((selectionSet: SelectionSetNode, s: Subschema) => {
     const resolver = mergedTypeInfo.resolvers.get(s);
-    const maybePromise = resolver(object, context, info, s, selectionSet);
-    resultMap.set(maybePromise, selectionSet);
+    let maybePromise = resolver(object, context, info, s, selectionSet);
     if (isPromise(maybePromise)) {
       containsPromises = true;
+      maybePromise = maybePromise.then(undefined, error => error);
     }
+    resultMap.set(maybePromise, selectionSet);
   });
 
   return containsPromises
diff --git a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts b/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts
@@ -27,7 +27,8 @@ export default class CheckResultAndHandleErrors implements Transform {
       delegationContext.fieldName,
       delegationContext.subschema,
       delegationContext.returnType,
-      delegationContext.skipTypeMerging
+      delegationContext.skipTypeMerging,
+      delegationContext.onLocatedError
     );
   }
 }
@@ -39,12 +40,14 @@ export function checkResultAndHandleErrors(
   responseKey: string = getResponseKeyFromInfo(info),
   subschema?: GraphQLSchema | SubschemaConfig,
   returnType: GraphQLOutputType = info.returnType,
-  skipTypeMerging?: boolean
+  skipTypeMerging?: boolean,
+  onLocatedError?: (originalError: GraphQLError) => GraphQLError
 ): any {
   const { data, unpathedErrors } = mergeDataAndErrors(
     result.data == null ? undefined : result.data[responseKey],
     result.errors == null ? [] : result.errors,
-    info ? responsePathAsArray(info.path) : undefined
+    info ? responsePathAsArray(info.path) : undefined,
+    onLocatedError
   );
 
   return resolveExternalValue(data, unpathedErrors, subschema, context, info, returnType, skipTypeMerging);
@@ -54,6 +57,7 @@ export function mergeDataAndErrors(
   data: any,
   errors: ReadonlyArray<GraphQLError>,
   path: Array<string | number>,
+  onLocatedError: (originalError: GraphQLError) => GraphQLError,
   index = 1
 ): { data: any; unpathedErrors: Array<GraphQLError> } {
   if (data == null) {
@@ -62,13 +66,16 @@ export function mergeDataAndErrors(
     }
 
     if (errors.length === 1) {
-      const error = errors[0];
+      const error = onLocatedError ? onLocatedError(errors[0]) : errors[0];
       const newPath =
         path === undefined ? error.path : error.path === undefined ? path : path.concat(error.path.slice(1));
+
       return { data: relocatedError(errors[0], newPath), unpathedErrors: [] };
     }
 
-    return { data: locatedError(new AggregateError(errors), undefined, path), unpathedErrors: [] };
+    const newError = locatedError(new AggregateError(errors), undefined, path);
+
+    return { data: newError, unpathedErrors: [] };
   }
 
   if (!errors.length) {
@@ -98,6 +105,7 @@ export function mergeDataAndErrors(
         data[pathSegment],
         errorMap[pathSegment],
         path,
+        onLocatedError,
         index + 1
       );
       data[pathSegment] = newData;
diff --git a/packages/delegate/src/types.ts b/packages/delegate/src/types.ts
@@ -51,6 +51,7 @@ export interface DelegationContext {
   context: Record<string, any>;
   info: GraphQLResolveInfo;
   returnType: GraphQLOutputType;
+  onLocatedError?: (originalError: GraphQLError) => GraphQLError;
   transforms: Array<Transform>;
   transformedSchema: GraphQLSchema;
   skipTypeMerging: boolean;
@@ -64,6 +65,7 @@ export interface IDelegateToSchemaOptions<TContext = Record<string, any>, TArgs
   operation?: OperationTypeNode;
   fieldName?: string;
   returnType?: GraphQLOutputType;
+  onLocatedError?: (originalError: GraphQLError) => GraphQLError;
   args?: TArgs;
   selectionSet?: SelectionSetNode;
   fieldNodes?: ReadonlyArray<FieldNode>;
diff --git a/packages/stitch/tests/mergeFailures.test.ts b/packages/stitch/tests/mergeFailures.test.ts
@@ -85,6 +85,64 @@ describe('merge failures', () => {
     expect(result).toEqual(expectedResult);
   });
 
+  test('proxies merged error arrays', async () => {
+    const schema1 = makeExecutableSchema({
+      typeDefs: `
+        type Thing {
+          id: ID!
+          name: String
+          desc: String
+        }
+        type Query {
+          things(ids: [ID!]!): [Thing]!
+        }
+      `,
+      resolvers: {
+        Query: {
+          things: () => [new Error('no thing')],
+        },
+      },
+    });
+
+    const schema2 = makeExecutableSchema({
+      typeDefs: `
+        type ParentThing {
+          thing: Thing
+        }
+        type Thing {
+          id: ID!
+        }
+        type Query {
+          parent: ParentThing
+        }
+      `,
+      resolvers: {
+        Query: {
+          parent: () => ({ thing: { id: 23 } }),
+        },
+      },
+    });
+
+    const stitchedSchema = stitchSchemas({
+      subschemas: [{
+        schema: schema1,
+        merge: {
+          Thing: {
+            selectionSet: '{ id }',
+            fieldName: 'things',
+            key: ({ id }) => id,
+            argsFromKeys: (ids) => ({ ids }),
+          },
+        }
+      }, {
+        schema: schema2,
+      }],
+    });
+
+    const stitchedResult = await graphql(stitchedSchema, '{ parent { thing { name desc id } } }');
+    expect(stitchedResult.errors[0].path).toEqual(['parent', 'thing', 'name']);
+  });
+
   it('proxies inappropriate null', async () => {
     const secondSchema = makeExecutableSchema({
       typeDefs: `
