diff --git a/packages/delegate/src/proxiedResult.ts b/packages/delegate/src/proxiedResult.ts index 21b1b323ef1..cf685bff438 100644 --- a/packages/delegate/src/proxiedResult.ts +++ b/packages/delegate/src/proxiedResult.ts @@ -1,6 +1,12 @@ -import { GraphQLError } from 'graphql'; - -import { mergeDeep, ERROR_SYMBOL, relocatedError, setErrors, getErrors } from '@graphql-tools/utils'; +import { + mergeDeep, + ERROR_SYMBOL, + relocatedError, + setErrors, + getErrors, + sliceRelativeError, + RelativeGraphQLError, +} from '@graphql-tools/utils'; import { handleNull } from './results/handleNull'; @@ -27,7 +33,7 @@ export function unwrapResult(parent: any, path: Array): any { setErrors( object, - errors.map(error => relocatedError(error, error.path != null ? error.path.slice(1) : undefined)) + errors.map(error => sliceRelativeError(error)) ); setObjectSubschema(object, subschema); @@ -51,15 +57,15 @@ export function dehoistResult(parent: any, delimeter = '__gqltf__'): any { obj[fieldName] = parent[alias]; }); - 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; + result[ERROR_SYMBOL] = parent[ERROR_SYMBOL].map((error: RelativeGraphQLError) => { + const path = error.relativePath.slice(); + const pathSegment = path.shift(); + const expandedPathSegment: Array = (pathSegment as string).split(delimeter); + return { + relativePath: expandedPathSegment.concat(path), + // 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]; diff --git a/packages/delegate/src/results/handleList.ts b/packages/delegate/src/results/handleList.ts index 3641e90aea8..95f44276f20 100644 --- a/packages/delegate/src/results/handleList.ts +++ b/packages/delegate/src/results/handleList.ts @@ -1,7 +1,6 @@ import { GraphQLList, GraphQLSchema, - GraphQLError, GraphQLResolveInfo, getNullableType, GraphQLType, @@ -10,7 +9,7 @@ import { isListType, } from 'graphql'; -import { getErrorsByPathSegment } from '@graphql-tools/utils'; +import { getErrorsByPathSegment, RelativeGraphQLError } from '@graphql-tools/utils'; import { handleNull } from './handleNull'; import { handleObject } from './handleObject'; @@ -19,7 +18,7 @@ import { SubschemaConfig } from '../types'; export function handleList( type: GraphQLList, list: Array, - errors: ReadonlyArray, + errors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, @@ -43,7 +42,7 @@ export function handleList( function handleListMember( type: GraphQLType, listMember: any, - errors: ReadonlyArray, + errors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, diff --git a/packages/delegate/src/results/handleNull.ts b/packages/delegate/src/results/handleNull.ts index b567ef7697a..ff80b6874b2 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'; +import { RelativeGraphQLError } 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.map(error => error.graphQLError)); } - const childErrors = getErrorsByPathSegment(errors); - - const result: Array = []; - Object.keys(childErrors).forEach(pathSegment => { - result.push(handleNull(childErrors[pathSegment])); - }); - - return result; + return errors[0].graphQLError; } return null; diff --git a/packages/delegate/src/results/handleObject.ts b/packages/delegate/src/results/handleObject.ts index 51d18e0d66a..0b7bc88cddf 100644 --- a/packages/delegate/src/results/handleObject.ts +++ b/packages/delegate/src/results/handleObject.ts @@ -1,6 +1,5 @@ import { GraphQLCompositeType, - GraphQLError, GraphQLSchema, isAbstractType, FieldNode, @@ -8,7 +7,13 @@ import { GraphQLResolveInfo, } from 'graphql'; -import { collectFields, GraphQLExecutionContext, setErrors, slicedError } from '@graphql-tools/utils'; +import { + collectFields, + GraphQLExecutionContext, + setErrors, + RelativeGraphQLError, + sliceRelativeError, +} from '@graphql-tools/utils'; import { setObjectSubschema, isSubschemaConfig } from '../Subschema'; import { mergeFields } from '../mergeFields'; import { MergedTypeInfo, SubschemaConfig } from '../types'; @@ -16,7 +21,7 @@ import { MergedTypeInfo, SubschemaConfig } from '../types'; export function handleObject( type: GraphQLCompositeType, object: any, - errors: ReadonlyArray, + errors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, @@ -26,7 +31,7 @@ export function handleObject( setErrors( object, - errors.map(error => slicedError(error)) + errors.map(error => sliceRelativeError(error)) ); setObjectSubschema(object, subschema); diff --git a/packages/delegate/src/results/handleResult.ts b/packages/delegate/src/results/handleResult.ts index 76cc53e5172..46fa8980a17 100644 --- a/packages/delegate/src/results/handleResult.ts +++ b/packages/delegate/src/results/handleResult.ts @@ -1,12 +1,6 @@ -import { - GraphQLResolveInfo, - getNullableType, - isCompositeType, - isLeafType, - isListType, - GraphQLError, - GraphQLSchema, -} from 'graphql'; +import { GraphQLResolveInfo, getNullableType, isCompositeType, isLeafType, isListType, GraphQLSchema } from 'graphql'; + +import { RelativeGraphQLError } from '@graphql-tools/utils'; import { SubschemaConfig } from '../types'; @@ -16,7 +10,7 @@ import { handleList } from './handleList'; export function handleResult( result: any, - errors: ReadonlyArray, + errors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, diff --git a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts b/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts index 351075e8cc1..e0909dbac9c 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 { Transform, getResponseKeyFromInfo } from '@graphql-tools/utils'; +import { Transform, getResponseKeyFromInfo, RelativeGraphQLError, toRelativeError } from '@graphql-tools/utils'; import { handleResult } from '../results/handleResult'; import { SubschemaConfig } from '../types'; @@ -50,7 +50,8 @@ export function checkResultAndHandleErrors( returnType: GraphQLOutputType = info.returnType, skipTypeMerging?: boolean ): any { - const errors = result.errors != null ? result.errors : []; + const errors: Array = + result.errors != null ? result.errors.map(error => toRelativeError(error, info)) : []; const data = result.data != null ? result.data[responseKey] : undefined; return handleResult(data, errors, 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..22d336727b4 100644 --- a/packages/utils/src/errors.ts +++ b/packages/utils/src/errors.ts @@ -1,7 +1,12 @@ -import { GraphQLError } from 'graphql'; +import { GraphQLError, responsePathAsArray, GraphQLResolveInfo } from 'graphql'; export const ERROR_SYMBOL = Symbol('subschemaErrors'); +export interface RelativeGraphQLError { + relativePath: Array; + graphQLError: GraphQLError; +} + export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray): GraphQLError { return new GraphQLError( originalError.message, @@ -18,28 +23,49 @@ export function slicedError(originalError: GraphQLError) { return relocatedError(originalError, originalError.path != null ? originalError.path.slice(1) : undefined); } -export function getErrorsByPathSegment(errors: ReadonlyArray): Record> { - const record = Object.create(null); +export function getErrorsByPathSegment( + errors: Array +): Record> { + const record: Record> = Object.create(null); errors.forEach(error => { - if (!error.path || error.path.length < 2) { + if (!error.relativePath || error.relativePath.length < 2) { return; } - const pathSegment = error.path[1]; + const pathSegment = error.relativePath[1]; - const current = pathSegment in record ? record[pathSegment] : []; - current.push(slicedError(error)); + const current: Array = pathSegment in record ? record[pathSegment] : []; + current.push({ + relativePath: error.relativePath.slice(1), + graphQLError: error.graphQLError, + }); record[pathSegment] = current; }); return record; } -export function setErrors(result: any, errors: Array) { +export function toRelativeError(error: Readonly, info: GraphQLResolveInfo): RelativeGraphQLError { + const relativePath = error.path?.slice() || []; + const sourcePath = info != null ? responsePathAsArray(info.path) : []; + return { + relativePath, + graphQLError: relocatedError(error, sourcePath.concat(relativePath.slice(1))), + }; +} + +export function sliceRelativeError(error: RelativeGraphQLError): RelativeGraphQLError { + return { + ...error, + relativePath: error.relativePath.slice(1), + }; +} + +export function setErrors(result: any, errors: Array) { result[ERROR_SYMBOL] = errors; } -export function getErrors(result: any, pathSegment: string): Array { +export function getErrors(result: any, pathSegment: string): Array { const errors = result != null ? result[ERROR_SYMBOL] : result; if (!Array.isArray(errors)) { @@ -49,7 +75,7 @@ export function getErrors(result: any, pathSegment: string): Array const fieldErrors = []; for (const error of errors) { - if (!error.path || error.path[0] === pathSegment) { + if (!error.relativePath || error.relativePath[0] === pathSegment) { fieldErrors.push(error); } }