diff --git a/packages/delegate/src/defaultMergedResolver.ts b/packages/delegate/src/defaultMergedResolver.ts index a023b1d3090..c8d3c0569bc 100644 --- a/packages/delegate/src/defaultMergedResolver.ts +++ b/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); } diff --git a/packages/delegate/src/proxiedResult.ts b/packages/delegate/src/proxiedResult.ts index 21b1b323ef1..0662010d184 100644 --- a/packages/delegate/src/proxiedResult.ts +++ b/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,6 +25,7 @@ export function unwrapResult(parent: any, path: Array): 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]; @@ -25,10 +33,13 @@ export function unwrapResult(parent: any, path: Array): any { 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 = (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 = (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, source: any) => { const subschema = source[OBJECT_SUBSCHEMA_SYMBOL]; Object.keys(source).forEach(key => { diff --git a/packages/delegate/src/results/handleList.ts b/packages/delegate/src/results/handleList.ts index 3641e90aea8..5aa5a03758a 100644 --- a/packages/delegate/src/results/handleList.ts +++ b/packages/delegate/src/results/handleList.ts @@ -19,19 +19,22 @@ import { SubschemaConfig } from '../types'; export function handleList( type: GraphQLList, list: Array, - errors: ReadonlyArray, + errors: Array, + depth: number, subschema: GraphQLSchema | SubschemaConfig, context: Record, 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, + errors: Array, + depth: number, subschema: GraphQLSchema | SubschemaConfig, context: Record, 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); } } diff --git a/packages/delegate/src/results/handleNull.ts b/packages/delegate/src/results/handleNull.ts index b567ef7697a..d01192337d2 100644 --- a/packages/delegate/src/results/handleNull.ts +++ b/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) { +export function handleNull(errors: Array) { 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 = []; - Object.keys(childErrors).forEach(pathSegment => { - result.push(handleNull(childErrors[pathSegment])); - }); - - return result; + return errors[0]; } return null; diff --git a/packages/delegate/src/results/handleObject.ts b/packages/delegate/src/results/handleObject.ts index 51d18e0d66a..dac94ea271c 100644 --- a/packages/delegate/src/results/handleObject.ts +++ b/packages/delegate/src/results/handleObject.ts @@ -8,7 +8,13 @@ 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'; @@ -16,7 +22,8 @@ import { MergedTypeInfo, SubschemaConfig } from '../types'; export function handleObject( type: GraphQLCompositeType, object: any, - errors: ReadonlyArray, + errors: Array, + depth: number, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, @@ -24,11 +31,10 @@ export function handleObject( ) { 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) { diff --git a/packages/delegate/src/results/handleResult.ts b/packages/delegate/src/results/handleResult.ts index 76cc53e5172..4ba9dce01c4 100644 --- a/packages/delegate/src/results/handleResult.ts +++ b/packages/delegate/src/results/handleResult.ts @@ -16,7 +16,8 @@ import { handleList } from './handleList'; export function handleResult( result: any, - errors: ReadonlyArray, + errors: Array, + depth: number, subschema: GraphQLSchema | SubschemaConfig, context: Record, 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); } } diff --git a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts b/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts index 351075e8cc1..77a6d7dcc35 100644 --- a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts +++ b/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); } diff --git a/packages/stitch/tests/errors.test.ts b/packages/stitch/tests/errors.test.ts index c0caf77f734..2154a74dfee 100644 --- a/packages/stitch/tests/errors.test.ts +++ b/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); + }); }); }); diff --git a/packages/utils/src/errors.ts b/packages/utils/src/errors.ts index 062993c8b53..829acfa3980 100644 --- a/packages/utils/src/errors.ts +++ b/packages/utils/src/errors.ts @@ -1,6 +1,17 @@ import { GraphQLError } from 'graphql'; export const ERROR_SYMBOL = Symbol('subschemaErrors'); +export const DEPTH_SYMBOL = Symbol('proxiedResultDepth'); + +export function toGraphQLErrors( + errors: ReadonlyArray, + sourcePath: Array +): Array { + return errors.map(error => { + const relativePath = error.path?.slice(1) || []; + return relocatedError(error, sourcePath.concat(relativePath)); + }); +} export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray): GraphQLError { return new GraphQLError( @@ -14,45 +25,41 @@ export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray ); } -export function slicedError(originalError: GraphQLError) { - return relocatedError(originalError, originalError.path != null ? originalError.path.slice(1) : undefined); -} +export function getErrorsByPathSegment( + errors: Array, + depth: number +): Record> { + return errors.reduce((acc, error) => { + const pathSegment = (error.path && error.path[depth]) ?? '__root'; -export function getErrorsByPathSegment(errors: ReadonlyArray): Record> { - const record = Object.create(null); - errors.forEach(error => { - if (!error.path || error.path.length < 2) { - return; + if (pathSegment in acc) { + acc[pathSegment].push(error); + } else { + acc[pathSegment] = [error]; } - const pathSegment = error.path[1]; - - const current = pathSegment in record ? record[pathSegment] : []; - current.push(slicedError(error)); - record[pathSegment] = current; - }); - - return record; + return acc; + }, Object.create(null)); } -export function setErrors(result: any, errors: Array) { - result[ERROR_SYMBOL] = errors; +export function setErrors(result: any, map: Record>) { + result[ERROR_SYMBOL] = map; } -export function getErrors(result: any, pathSegment: string): Array { - const errors = result != null ? result[ERROR_SYMBOL] : result; +export function getErrors(result: any, pathSegment: string | number): Array { + const proxiedErrors: Record> = result[ERROR_SYMBOL]; - if (!Array.isArray(errors)) { + if (proxiedErrors == null) { return null; } - const fieldErrors = []; + return proxiedErrors[pathSegment] ?? []; +} - for (const error of errors) { - if (!error.path || error.path[0] === pathSegment) { - fieldErrors.push(error); - } - } +export function setDepth(result: any, depth: number) { + result[DEPTH_SYMBOL] = depth; +} - return fieldErrors; +export function getDepth(result: any): number { + return result[DEPTH_SYMBOL]; } diff --git a/packages/wrap/src/generateProxyingResolvers.ts b/packages/wrap/src/generateProxyingResolvers.ts index 6581f31a9d5..49c2f2ee60b 100644 --- a/packages/wrap/src/generateProxyingResolvers.ts +++ b/packages/wrap/src/generateProxyingResolvers.ts @@ -1,6 +1,13 @@ import { GraphQLSchema, GraphQLFieldResolver, GraphQLObjectType, GraphQLResolveInfo } from 'graphql'; -import { Transform, Operation, getResponseKeyFromInfo, getErrors, applySchemaTransforms } from '@graphql-tools/utils'; +import { + Transform, + Operation, + getResponseKeyFromInfo, + getErrors, + applySchemaTransforms, + getDepth, +} from '@graphql-tools/utils'; import { delegateToSchema, getSubschema, @@ -91,12 +98,13 @@ function createPossiblyNestedProxyingResolver( // Check to see if the parent contains a proxied result if (errors != null) { const subschema = getSubschema(parent, responseKey); + const depth = getDepth(parent); // If there is a proxied result from this subschema, return it // This can happen even for a root field when the root type ia // also nested as a field within a different type. if (subschemaOrSubschemaConfig === subschema && parent[responseKey] !== undefined) { - return handleResult(parent[responseKey], errors, subschema, context, info); + return handleResult(parent[responseKey], errors, depth, subschema, context, info); } } } diff --git a/packages/wrap/tests/errors.test.ts b/packages/wrap/tests/errors.test.ts index 728e6677823..43191ace2ff 100644 --- a/packages/wrap/tests/errors.test.ts +++ b/packages/wrap/tests/errors.test.ts @@ -30,13 +30,18 @@ describe('Errors', () => { const error = { message: 'Test error without path', }; + const relativeError = { + graphQLError: error, + }; const mockErrors: any = { - responseKey: '', - [ERROR_SYMBOL]: [error], + responseKey: null, + [ERROR_SYMBOL]: { + 'responseKey': [relativeError], + }, }; expect(getErrors(mockErrors, 'responseKey')).toEqual([ - mockErrors[ERROR_SYMBOL][0], + mockErrors[ERROR_SYMBOL].responseKey[0], ]); }); });