diff --git a/packages/delegate/src/defaultMergedResolver.ts b/packages/delegate/src/defaultMergedResolver.ts index 0b42667d38e..bfbe860715e 100644 --- a/packages/delegate/src/defaultMergedResolver.ts +++ b/packages/delegate/src/defaultMergedResolver.ts @@ -4,7 +4,7 @@ import { getResponseKeyFromInfo } from '@graphql-tools/utils'; import { resolveExternalValue } from './resolveExternalValue'; import { getSubschema } from './Subschema'; -import { getErrors, isExternalData } from './externalData'; +import { getUnpathedErrors, isExternalData } from './externalData'; import { ExternalData } from './types'; /** @@ -32,8 +32,8 @@ export function defaultMergedResolver( } const data = parent[responseKey]; + const unpathedErrors = getUnpathedErrors(parent); const subschema = getSubschema(parent, responseKey); - const errors = getErrors(parent, responseKey); - return resolveExternalValue(data, errors, subschema, context, info); + return resolveExternalValue(data, unpathedErrors, subschema, context, info); } diff --git a/packages/delegate/src/externalData.ts b/packages/delegate/src/externalData.ts index 97a17120ad9..05ed7e8ae21 100644 --- a/packages/delegate/src/externalData.ts +++ b/packages/delegate/src/externalData.ts @@ -1,19 +1,12 @@ import { GraphQLSchema, GraphQLError, GraphQLObjectType, SelectionSetNode } from 'graphql'; -import { - slicedError, - extendedError, - mergeDeep, - relocatedError, - GraphQLExecutionContext, - collectFields, -} from '@graphql-tools/utils'; +import { mergeDeep, relocatedError, GraphQLExecutionContext, collectFields } from '@graphql-tools/utils'; import { SubschemaConfig, ExternalData } from './types'; -import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL, ERROR_SYMBOL } from './symbols'; +import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL, UNPATHED_ERRORS_SYMBOL } from './symbols'; export function isExternalData(data: any): data is ExternalData { - return data[ERROR_SYMBOL] !== undefined; + return data[UNPATHED_ERRORS_SYMBOL] !== undefined; } export function annotateExternalData( @@ -24,7 +17,7 @@ export function annotateExternalData( Object.defineProperties(data, { [OBJECT_SUBSCHEMA_SYMBOL]: { value: subschema }, [FIELD_SUBSCHEMA_MAP_SYMBOL]: { value: Object.create(null) }, - [ERROR_SYMBOL]: { value: errors }, + [UNPATHED_ERRORS_SYMBOL]: { value: errors }, }); return data; } @@ -33,39 +26,8 @@ export function getSubschema(data: ExternalData, responseKey: string): GraphQLSc return data[FIELD_SUBSCHEMA_MAP_SYMBOL][responseKey] ?? data[OBJECT_SUBSCHEMA_SYMBOL]; } -export function getErrors(data: ExternalData, pathSegment: string): Array { - const errors = data == null ? data : data[ERROR_SYMBOL]; - - if (!Array.isArray(errors)) { - return null; - } - - const fieldErrors = []; - - for (const error of errors) { - if (!error.path || error.path[0] === pathSegment) { - fieldErrors.push(error); - } - } - - return fieldErrors; -} - -export function getErrorsByPathSegment(errors: ReadonlyArray): Record> { - const record = Object.create(null); - errors.forEach(error => { - if (!error.path || error.path.length < 2) { - return; - } - - const pathSegment = error.path[1]; - - const current = pathSegment in record ? record[pathSegment] : []; - current.push(slicedError(error)); - record[pathSegment] = current; - }); - - return record; +export function getUnpathedErrors(data: ExternalData): Array { + return data[UNPATHED_ERRORS_SYMBOL]; } export function mergeExternalData( @@ -95,12 +57,11 @@ export function mergeExternalData( ); const nullResult = {}; Object.keys(fieldNodes).forEach(responseKey => { - errors.push(relocatedError(source, [responseKey])); - nullResult[responseKey] = null; + nullResult[responseKey] = relocatedError(source, path.concat([responseKey])); }); results.push(nullResult); } else { - errors = errors.concat(source[ERROR_SYMBOL]); + errors = errors.concat(source[UNPATHED_ERRORS_SYMBOL]); results.push(source); } }); @@ -118,14 +79,7 @@ export function mergeExternalData( ? Object.assign({}, target[FIELD_SUBSCHEMA_MAP_SYMBOL], fieldSubschemaMap) : fieldSubschemaMap; - const annotatedErrors = errors.map(error => { - return extendedError(error, { - ...error.extensions, - graphQLToolsMergedPath: error.path != null ? [...path, ...error.path] : path, - }); - }); - - result[ERROR_SYMBOL] = target[ERROR_SYMBOL].concat(annotatedErrors); + result[UNPATHED_ERRORS_SYMBOL] = target[UNPATHED_ERRORS_SYMBOL].concat(errors); return result; } diff --git a/packages/delegate/src/resolveExternalValue/handleList.ts b/packages/delegate/src/resolveExternalValue/handleList.ts index e64d642f77c..9c2395955dd 100644 --- a/packages/delegate/src/resolveExternalValue/handleList.ts +++ b/packages/delegate/src/resolveExternalValue/handleList.ts @@ -11,7 +11,6 @@ import { } from 'graphql'; import { SubschemaConfig } from '../types'; -import { getErrorsByPathSegment } from '../externalData'; import { handleNull } from './handleNull'; import { handleObject } from './handleObject'; @@ -19,19 +18,17 @@ import { handleObject } from './handleObject'; export function handleList( type: GraphQLList, list: Array, - errors: ReadonlyArray, + unpathedErrors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, skipTypeMerging?: boolean ) { - const childErrors = getErrorsByPathSegment(errors); - - return list.map((listMember, index) => + return list.map(listMember => handleListMember( getNullableType(type.ofType), listMember, - index in childErrors ? childErrors[index] : [], + unpathedErrors, subschema, context, info, @@ -43,21 +40,25 @@ export function handleList( function handleListMember( type: GraphQLType, listMember: any, - errors: ReadonlyArray, + unpathedErrors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, skipTypeMerging?: boolean ): any { + if (listMember instanceof Error) { + return listMember; + } + if (listMember == null) { - return handleNull(errors); + return handleNull(unpathedErrors); } if (isLeafType(type)) { return type.parseValue(listMember); } else if (isCompositeType(type)) { - return handleObject(type, listMember, errors, subschema, context, info, skipTypeMerging); + return handleObject(type, listMember, unpathedErrors, subschema, context, info, skipTypeMerging); } else if (isListType(type)) { - return handleList(type, listMember, errors, subschema, context, info, skipTypeMerging); + return handleList(type, listMember, unpathedErrors, subschema, context, info, skipTypeMerging); } } diff --git a/packages/delegate/src/resolveExternalValue/handleNull.ts b/packages/delegate/src/resolveExternalValue/handleNull.ts index 52ecd2b68d7..f1d4afbcc08 100644 --- a/packages/delegate/src/resolveExternalValue/handleNull.ts +++ b/packages/delegate/src/resolveExternalValue/handleNull.ts @@ -1,28 +1,27 @@ -import { GraphQLError } from 'graphql'; +import { GraphQLError, locatedError } from 'graphql'; import AggregateError from '@ardatan/aggregate-error'; -import { relocatedError, unextendedError } from '@graphql-tools/utils'; +const reportedErrors: WeakMap = new Map(); -export function handleNull(errors: ReadonlyArray) { - if (errors.length) { - const graphQLToolsMergedPath = errors[0].extensions.graphQLToolsMergedPath; - const unannotatedErrors = errors.map(error => unextendedError(error, 'graphQLToolsMergedPath')); +export function handleNull(unpathedErrors: Array) { + if (unpathedErrors.length) { + const unreportedErrors: Array = []; + unpathedErrors.forEach(error => { + if (!reportedErrors.has(error)) { + unreportedErrors.push(error); + reportedErrors.set(error, true); + } + }); - if (unannotatedErrors.length > 1) { - const combinedError = new AggregateError(unannotatedErrors); - return new GraphQLError( - combinedError.message, - undefined, - undefined, - undefined, - graphQLToolsMergedPath, - combinedError - ); - } + if (unreportedErrors.length) { + if (unreportedErrors.length === 1) { + return unreportedErrors[0]; + } - const error = unannotatedErrors[0]; - return relocatedError(error, graphQLToolsMergedPath); + const combinedError = new AggregateError(unreportedErrors); + return locatedError(combinedError, undefined, unreportedErrors[0].path); + } } return null; diff --git a/packages/delegate/src/resolveExternalValue/handleObject.ts b/packages/delegate/src/resolveExternalValue/handleObject.ts index ebd24738264..2604fb1b354 100644 --- a/packages/delegate/src/resolveExternalValue/handleObject.ts +++ b/packages/delegate/src/resolveExternalValue/handleObject.ts @@ -1,7 +1,5 @@ import { GraphQLCompositeType, GraphQLError, GraphQLSchema, isAbstractType, GraphQLResolveInfo } from 'graphql'; -import { slicedError } from '@graphql-tools/utils'; - import { SubschemaConfig } from '../types'; import { annotateExternalData } from '../externalData'; @@ -11,7 +9,7 @@ import { getFieldsNotInSubschema } from './getFieldsNotInSubschema'; export function handleObject( type: GraphQLCompositeType, object: any, - errors: ReadonlyArray, + unpathedErrors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, @@ -19,11 +17,7 @@ export function handleObject( ) { const stitchingInfo = info?.schema.extensions?.stitchingInfo; - annotateExternalData( - object, - errors.map(error => slicedError(error)), - subschema - ); + annotateExternalData(object, unpathedErrors, subschema); if (skipTypeMerging || !stitchingInfo) { return object; diff --git a/packages/delegate/src/resolveExternalValue/index.ts b/packages/delegate/src/resolveExternalValue/index.ts index 3b49020313d..a6afbb60d53 100644 --- a/packages/delegate/src/resolveExternalValue/index.ts +++ b/packages/delegate/src/resolveExternalValue/index.ts @@ -6,7 +6,6 @@ import { isListType, GraphQLError, GraphQLSchema, - responsePathAsArray, } from 'graphql'; import { SubschemaConfig } from '../types'; @@ -14,43 +13,31 @@ import { SubschemaConfig } from '../types'; import { handleNull } from './handleNull'; import { handleObject } from './handleObject'; import { handleList } from './handleList'; -import { extendedError } from '@graphql-tools/utils'; export function resolveExternalValue( result: any, - errors: ReadonlyArray, + unpathedErrors: Array, subschema: GraphQLSchema | SubschemaConfig, context: Record, info: GraphQLResolveInfo, 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 instanceof Error) { + return result; + } + if (result == null) { - return handleNull(annotatedErrors); + return handleNull(unpathedErrors); } if (isLeafType(type)) { return type.parseValue(result); } else if (isCompositeType(type)) { - return handleObject(type, result, annotatedErrors, subschema, context, info, skipTypeMerging); + return handleObject(type, result, unpathedErrors, subschema, context, info, skipTypeMerging); } else if (isListType(type)) { - return handleList(type, result, annotatedErrors, subschema, context, info, skipTypeMerging); + return handleList(type, result, unpathedErrors, subschema, context, info, skipTypeMerging); } } diff --git a/packages/delegate/src/symbols.ts b/packages/delegate/src/symbols.ts index 30782cf8867..dad8b7b958a 100644 --- a/packages/delegate/src/symbols.ts +++ b/packages/delegate/src/symbols.ts @@ -1,3 +1,3 @@ -export const ERROR_SYMBOL = Symbol('subschemaErrors'); +export const UNPATHED_ERRORS_SYMBOL = Symbol('subschemaErrors'); export const OBJECT_SUBSCHEMA_SYMBOL = Symbol('initialSubschema'); export const FIELD_SUBSCHEMA_MAP_SYMBOL = Symbol('subschemaMap'); diff --git a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts b/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts index 16c17fd459d..1677a494da6 100644 --- a/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts +++ b/packages/delegate/src/transforms/CheckResultAndHandleErrors.ts @@ -1,8 +1,18 @@ -import { GraphQLResolveInfo, GraphQLOutputType, GraphQLSchema } from 'graphql'; +import { + GraphQLResolveInfo, + GraphQLOutputType, + GraphQLSchema, + GraphQLError, + responsePathAsArray, + locatedError, +} from 'graphql'; + +import AggregateError from '@ardatan/aggregate-error'; + +import { getResponseKeyFromInfo, ExecutionResult, relocatedError } from '@graphql-tools/utils'; -import { getResponseKeyFromInfo, ExecutionResult } from '@graphql-tools/utils'; -import { resolveExternalValue } from '../resolveExternalValue'; import { SubschemaConfig, Transform, DelegationContext } from '../types'; +import { resolveExternalValue } from '../resolveExternalValue'; export default class CheckResultAndHandleErrors implements Transform { private readonly context?: Record; @@ -54,8 +64,71 @@ export function checkResultAndHandleErrors( returnType: GraphQLOutputType = info.returnType, skipTypeMerging?: boolean ): any { - const errors = result.errors != null ? result.errors : []; - const data = result.data != null ? result.data[responseKey] : undefined; + const { data, unpathedErrors } = mergeDataAndErrors( + result.data == null ? undefined : result.data[responseKey], + result.errors == null ? [] : result.errors, + info ? responsePathAsArray(info.path) : undefined + ); + + return resolveExternalValue(data, unpathedErrors, subschema, context, info, returnType, skipTypeMerging); +} + +export function mergeDataAndErrors( + data: any, + errors: ReadonlyArray, + path: Array, + index = 1 +): { data: any; unpathedErrors: Array } { + if (data == null) { + if (!errors.length) { + return { data: null, unpathedErrors: [] }; + } + + if (errors.length === 1) { + const error = 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: [] }; + } + + if (!errors.length) { + return { data, unpathedErrors: [] }; + } + + let unpathedErrors: Array = []; + + const errorMap: Record> = Object.create(null); + errors.forEach(error => { + const pathSegment = error.path?.[index]; + if (pathSegment != null) { + const pathSegmentErrors = errorMap[pathSegment]; + if (pathSegmentErrors === undefined) { + errorMap[pathSegment] = [error]; + } else { + pathSegmentErrors.push(error); + } + } else { + unpathedErrors.push(error); + } + }); + + Object.keys(errorMap).forEach(pathSegment => { + if (data[pathSegment] !== undefined) { + const { data: newData, unpathedErrors: newErrors } = mergeDataAndErrors( + data[pathSegment], + errorMap[pathSegment], + path, + index + 1 + ); + data[pathSegment] = newData; + unpathedErrors = unpathedErrors.concat(newErrors); + } else { + unpathedErrors = unpathedErrors.concat(errorMap[pathSegment]); + } + }); - return resolveExternalValue(data, errors, subschema, context, info, returnType, skipTypeMerging); + return { data, unpathedErrors }; } diff --git a/packages/delegate/src/types.ts b/packages/delegate/src/types.ts index b2d6e7096c7..0ba322b8ee0 100644 --- a/packages/delegate/src/types.ts +++ b/packages/delegate/src/types.ts @@ -16,7 +16,7 @@ import { import DataLoader from 'dataloader'; -import { Operation, Request, TypeMap, ExecutionResult, ERROR_SYMBOL } from '@graphql-tools/utils'; +import { Operation, Request, TypeMap, ExecutionResult, UNPATHED_ERRORS_SYMBOL } from '@graphql-tools/utils'; import { Subschema } from './Subschema'; import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL } from './symbols'; @@ -203,5 +203,5 @@ export interface ExternalData { key: any; [OBJECT_SUBSCHEMA_SYMBOL]: GraphQLSchema | SubschemaConfig; [FIELD_SUBSCHEMA_MAP_SYMBOL]: Record; - [ERROR_SYMBOL]: Array; + [UNPATHED_ERRORS_SYMBOL]: Array; } diff --git a/packages/delegate/tests/errors.test.ts b/packages/delegate/tests/errors.test.ts index 30fe7d96b18..755713c411b 100644 --- a/packages/delegate/tests/errors.test.ts +++ b/packages/delegate/tests/errors.test.ts @@ -1,8 +1,13 @@ -import { GraphQLError, GraphQLResolveInfo } from 'graphql'; +import { GraphQLError, GraphQLResolveInfo, locatedError, graphql } from 'graphql'; + +import { makeExecutableSchema } from '@graphql-tools/schema'; +import { ExecutionResult } from '@graphql-tools/utils'; +import { stitchSchemas } from '@graphql-tools/stitch'; import { checkResultAndHandleErrors } from '../src/transforms/CheckResultAndHandleErrors'; -import { getErrors } from '../src/externalData'; -import { ERROR_SYMBOL } from '../src/symbols'; +import { UNPATHED_ERRORS_SYMBOL } from '../src/symbols'; +import { getUnpathedErrors } from '../src/externalData'; +import { delegateToSchema, defaultMergedResolver } from '../src'; class ErrorWithExtensions extends GraphQLError { constructor(message: string, code: string) { @@ -11,18 +16,18 @@ class ErrorWithExtensions extends GraphQLError { } describe('Errors', () => { - describe('getErrors', () => { - test('should return all errors including if path is not defined', () => { + describe('getUnpathedErrors', () => { + test('should return all unpathed errors', () => { const error = { message: 'Test error without path', }; - const mockErrors: any = { + const mockExternalData: any = { responseKey: '', - [ERROR_SYMBOL]: [error], + [UNPATHED_ERRORS_SYMBOL]: [error], }; - expect(getErrors(mockErrors, 'responseKey')).toEqual([ - mockErrors[ERROR_SYMBOL][0], + expect(getUnpathedErrors(mockExternalData)).toEqual([ + mockExternalData[UNPATHED_ERRORS_SYMBOL][0], ]); }); }); @@ -84,5 +89,124 @@ describe('Errors', () => { }); } }); + + // see https://github.com/ardatan/graphql-tools/issues/1641 + describe('it proxies errors with invalid paths', () => { + test('it works with bare delegation', async () => { + const typeDefs = ` + type Object { + field1: String + field2: String + } + type Query { + object: Object + } + `; + + const unpathedError = locatedError(new Error('TestError'), undefined, ["_entities", 7, "name"]); + + const remoteSchema = makeExecutableSchema({ + typeDefs, + resolvers: { + Query: { + object: () => ({ + field1: unpathedError, + field2: 'data', + }) + } + } + }); + + const gatewaySchema = makeExecutableSchema({ + typeDefs, + resolvers: { + Query: { + object: (_parent, _args, context, info) => delegateToSchema({ + schema: remoteSchema, + operation: 'query', + context, + info, + }), + } + }, + }); + + const query = `{ + object { + field1 + field2 + } + }`; + + const expectedResult: ExecutionResult = { + data: { + object: { + field1: null, + field2: 'data', + } + }, + errors: [unpathedError], + }; + + const gatewayResult = await graphql({ + schema: gatewaySchema, + source: query, + fieldResolver: defaultMergedResolver, + }); + + expect(gatewayResult).toEqual(expectedResult); + }); + + test('it works with stitched schemas', async () => { + const typeDefs = ` + type Object { + field1: String + field2: String + } + type Query { + object: Object + } + `; + + const unpathedError = locatedError(new Error('TestError'), undefined, ["_entities", 7, "name"]); + + const remoteSchema = makeExecutableSchema({ + typeDefs, + resolvers: { + Query: { + object: () => ({ + field1: unpathedError, + field2: 'data', + }) + } + } + }); + + const gatewaySchema = stitchSchemas({ + subschemas: [remoteSchema], + }); + + const query = `{ + object { + field1 + field2 + } + }`; + + const expectedResult: ExecutionResult = { + data: { + object: { + field1: null, + field2: 'data', + } + }, + errors: [unpathedError], + }; + + const gatewayResult = await graphql(gatewaySchema, query); + + expect(gatewayResult).toEqual(expectedResult); + }); + }); }); }); diff --git a/packages/utils/src/errors.ts b/packages/utils/src/errors.ts index 1c26a7d39af..27693349367 100644 --- a/packages/utils/src/errors.ts +++ b/packages/utils/src/errors.ts @@ -11,64 +11,3 @@ export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray originalError.extensions ); } - -export function extendedError(originalError: GraphQLError, extensions: Record): 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); -} diff --git a/packages/wrap/src/generateProxyingResolvers.ts b/packages/wrap/src/generateProxyingResolvers.ts index 8f34f7378fa..4934e0e4e9d 100644 --- a/packages/wrap/src/generateProxyingResolvers.ts +++ b/packages/wrap/src/generateProxyingResolvers.ts @@ -12,7 +12,7 @@ import { Transform, applySchemaTransforms, isExternalData, - getErrors, + getUnpathedErrors, } from '@graphql-tools/delegate'; export function generateProxyingResolvers( @@ -93,14 +93,14 @@ function createPossiblyNestedProxyingResolver( // Check to see if the parent contains a proxied result if (isExternalData(parent)) { + const unpathedErrors = getUnpathedErrors(parent); const subschema = getSubschema(parent, responseKey); - const errors = getErrors(parent, responseKey); // 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 resolveExternalValue(parent[responseKey], errors, subschema, context, info); + return resolveExternalValue(parent[responseKey], unpathedErrors, subschema, context, info); } } }