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

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)
ardatan · Nov 29, 2020 · 29ead57 · 29ead57
1 parent 302228f
commit 29ead57
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 7 deletions.
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: `