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(errors): preserve merged paths #1875

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions packages/delegate/src/createMergedResolver.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
Expand Down
42 changes: 17 additions & 25 deletions packages/delegate/src/results/handleNull.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
23 changes: 20 additions & 3 deletions packages/delegate/src/results/handleResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
}
12 changes: 6 additions & 6 deletions packages/delegate/src/results/mergeFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,21 +166,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,
Expand All @@ -191,7 +191,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,
Expand Down
59 changes: 54 additions & 5 deletions packages/delegate/src/results/mergeProxiedResults.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,53 @@
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 {
const results = sources.filter(source => !(source instanceof Error));
export function mergeProxiedResults(
info: GraphQLResolveInfo,
target: any,
sources: Array<any>,
selectionSets: Array<SelectionSetNode>
): any {
const results: Array<any> = [];
let errors: Array<GraphQLError> = [];

const path = responsePathAsArray(info.path);

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;
});
} else {
errors = errors.concat(source[ERROR_SYMBOL]);
results.push(source);
}
});

const fieldSubschemaMap = results.reduce((acc: Record<any, SubschemaConfig>, source: any) => {
const subschema = source[OBJECT_SUBSCHEMA_SYMBOL];
Object.keys(source).forEach(key => {
Expand All @@ -18,8 +61,14 @@ export function mergeProxiedResults(target: any, ...sources: Array<any>): any {
? Object.assign({}, target[FIELD_SUBSCHEMA_MAP_SYMBOL], fieldSubschemaMap)
: fieldSubschemaMap;

const errors = sources.map((source: any) => (source instanceof Error ? source : source[ERROR_SYMBOL]));
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;
}
35 changes: 35 additions & 0 deletions packages/stitch/tests/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
});
});
});

Expand Down
57 changes: 57 additions & 0 deletions packages/utils/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down