Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(stitch) fix merged error paths from batched arrays #2288

Merged
merged 8 commits into from
Nov 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/seven-dryers-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-tools/batch-delegate': patch
'@graphql-tools/delegate': patch
---

fix(batch-delegate): proxy batched errors
7 changes: 7 additions & 0 deletions packages/batch-delegate/src/getLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions packages/delegate/src/delegateToSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export function delegateRequest({
fieldName,
args,
returnType,
onLocatedError,
context,
transforms = [],
transformedSchema,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions packages/delegate/src/mergeFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions packages/delegate/src/transforms/CheckResultAndHandleErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export default class CheckResultAndHandleErrors implements Transform {
delegationContext.fieldName,
delegationContext.subschema,
delegationContext.returnType,
delegationContext.skipTypeMerging
delegationContext.skipTypeMerging,
delegationContext.onLocatedError
);
}
}
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -98,6 +105,7 @@ export function mergeDataAndErrors(
data[pathSegment],
errorMap[pathSegment],
path,
onLocatedError,
index + 1
);
data[pathSegment] = newData;
Expand Down
2 changes: 2 additions & 0 deletions packages/delegate/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>;
Expand Down
58 changes: 58 additions & 0 deletions packages/stitch/tests/mergeFailures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down