From 9c8c6d1dc80831c8d17a312ddadf8e5ecc270c68 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 24 Sep 2025 17:34:24 +0300 Subject: [PATCH 1/5] fix(batch-delegate): correct error paths in case of batch delegation --- .changeset/breezy-weeks-camp.md | 6 + .../src/batchDelegateToSchema.ts | 14 +- .../batch-delegate/tests/errorPaths.test.ts | 162 ++++++++++++++++++ packages/delegate/src/resolveExternalValue.ts | 4 +- 4 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 .changeset/breezy-weeks-camp.md create mode 100644 packages/batch-delegate/tests/errorPaths.test.ts diff --git a/.changeset/breezy-weeks-camp.md b/.changeset/breezy-weeks-camp.md new file mode 100644 index 000000000..7e2384ddf --- /dev/null +++ b/.changeset/breezy-weeks-camp.md @@ -0,0 +1,6 @@ +--- +'@graphql-tools/batch-delegate': patch +'@graphql-tools/delegate': patch +--- + +Correct error paths in case of batch delegation with the same error diff --git a/packages/batch-delegate/src/batchDelegateToSchema.ts b/packages/batch-delegate/src/batchDelegateToSchema.ts index 71bd6f921..e65e232cb 100644 --- a/packages/batch-delegate/src/batchDelegateToSchema.ts +++ b/packages/batch-delegate/src/batchDelegateToSchema.ts @@ -1,5 +1,8 @@ +import { handleMaybePromise } from '@whatwg-node/promise-helpers'; +import { GraphQLError } from 'graphql'; import { getLoader } from './getLoader.js'; import { BatchDelegateOptions } from './types.js'; +import { pathToArray, relocatedError } from '@graphql-tools/utils'; export function batchDelegateToSchema( options: BatchDelegateOptions, @@ -11,5 +14,14 @@ export function batchDelegateToSchema( return []; } const loader = getLoader(options); - return Array.isArray(key) ? loader.loadMany(key) : loader.load(key); + return handleMaybePromise( + () => (Array.isArray(key) ? loader.loadMany(key) : loader.load(key)), + res => res, + error => { + if (options.info?.path && error instanceof GraphQLError) { + return relocatedError(error, pathToArray(options.info.path)); + } + return error; + } + ); } diff --git a/packages/batch-delegate/tests/errorPaths.test.ts b/packages/batch-delegate/tests/errorPaths.test.ts new file mode 100644 index 000000000..a4060773a --- /dev/null +++ b/packages/batch-delegate/tests/errorPaths.test.ts @@ -0,0 +1,162 @@ +import { GraphQLError, parse } from 'graphql'; +import { batchDelegateToSchema } from '@graphql-tools/batch-delegate'; +import { delegateToSchema } from '@graphql-tools/delegate'; +import { makeExecutableSchema } from '@graphql-tools/schema'; +import { stitchSchemas } from '@graphql-tools/stitch'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { normalizedExecutor } from '@graphql-tools/executor'; + +class NotFoundError extends GraphQLError { + constructor(id: unknown) { + super('Not Found', { + extensions: { id }, + }) + } +} + +describe('preserves error path indices', () => { + const getProperty = vi.fn((id: unknown) => { + return new NotFoundError(id); + }); + + beforeEach(() => { + getProperty.mockClear(); + }); + + const subschema = makeExecutableSchema({ + typeDefs: /* GraphQL */ ` + type Property { + id: ID! + } + + type Object { + id: ID! + propertyId: ID! + } + + type Query { + objects: [Object!]! + propertyById(id: ID!): Property + propertiesByIds(ids: [ID!]!): [Property]! + } + `, + resolvers: { + Query: { + objects: () => { + return [ + { id: '1', propertyId: '1' }, + { id: '2', propertyId: '1' }, + ]; + }, + propertyById: (_, args) => getProperty(args.id), + propertiesByIds: (_, args) => args.ids.map(getProperty), + }, + }, + }); + + const subschemas = [subschema]; + const typeDefs = /* GraphQL */ ` + extend type Object { + property: Property + } + `; + + const query = /* GraphQL */ ` + query { + objects { + id + property { + id + } + } + } + `; + + const expected = { + errors: [ + { + message: 'Not Found', + extensions: { id: '1' }, + path: ['objects', 0, 'property'], + }, + { + message: 'Not Found', + extensions: { id: '1' }, + path: ['objects', 1, 'property'], + }, + ], + data: { + objects: [ + { + id: '1', + property: null as null, + }, + { + id: '2', + property: null as null, + }, + ], + }, + }; + + test('using delegateToSchema', async () => { + const schema = stitchSchemas({ + subschemas, + typeDefs, + resolvers: { + Object: { + property: { + selectionSet: '{ propertyId }', + resolve: (source, _, context, info) => { + return delegateToSchema({ + schema: subschema, + fieldName: 'propertyById', + args: { id: source.propertyId }, + context, + info, + }); + }, + }, + }, + }, + }); + + + const result = await normalizedExecutor({ + schema, + document: parse(query), + }) + + expect(getProperty).toBeCalledTimes(2); + expect(result).toMatchObject(expected); + }); + + test('using batchDelegateToSchema', async () => { + const schema = stitchSchemas({ + subschemas, + typeDefs, + resolvers: { + Object: { + property: { + selectionSet: '{ propertyId }', + resolve: (source, _, context, info) => batchDelegateToSchema({ + schema: subschema, + fieldName: 'propertiesByIds', + key: source.propertyId, + context, + info, + }), + }, + }, + }, + }); + + const result = await normalizedExecutor({ + schema, + document: parse(query), + }) + + expect(getProperty).toBeCalledTimes(1); + expect(result).toMatchObject(expected); + }); +}); \ No newline at end of file diff --git a/packages/delegate/src/resolveExternalValue.ts b/packages/delegate/src/resolveExternalValue.ts index 8aa6958c5..9402afaa4 100644 --- a/packages/delegate/src/resolveExternalValue.ts +++ b/packages/delegate/src/resolveExternalValue.ts @@ -182,7 +182,7 @@ function resolveExternalList>( ); } -const reportedErrors = new WeakMap(); +const reportedErrors = new WeakSet(); function reportUnpathedErrorsViaNull(unpathedErrors: Array) { if (unpathedErrors.length) { @@ -190,7 +190,7 @@ function reportUnpathedErrorsViaNull(unpathedErrors: Array) { for (const error of unpathedErrors) { if (!reportedErrors.has(error)) { unreportedErrors.push(error); - reportedErrors.set(error, true); + reportedErrors.add(error); } } From 98cce8528caa75fa99f9dc33cea2e6be41bcbea1 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 24 Sep 2025 17:38:13 +0300 Subject: [PATCH 2/5] Update packages/batch-delegate/tests/errorPaths.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- packages/batch-delegate/tests/errorPaths.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/batch-delegate/tests/errorPaths.test.ts b/packages/batch-delegate/tests/errorPaths.test.ts index a4060773a..189c86b38 100644 --- a/packages/batch-delegate/tests/errorPaths.test.ts +++ b/packages/batch-delegate/tests/errorPaths.test.ts @@ -89,11 +89,11 @@ describe('preserves error path indices', () => { objects: [ { id: '1', - property: null as null, + property: null, }, { id: '2', - property: null as null, + property: null, }, ], }, From 07fc37663b579b1b9bd14dba51f49c127398c5b9 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 24 Sep 2025 17:35:58 +0300 Subject: [PATCH 3/5] Format --- .../src/batchDelegateToSchema.ts | 8 +- .../batch-delegate/tests/errorPaths.test.ts | 224 +++++++++--------- 2 files changed, 116 insertions(+), 116 deletions(-) diff --git a/packages/batch-delegate/src/batchDelegateToSchema.ts b/packages/batch-delegate/src/batchDelegateToSchema.ts index e65e232cb..1c882ea4c 100644 --- a/packages/batch-delegate/src/batchDelegateToSchema.ts +++ b/packages/batch-delegate/src/batchDelegateToSchema.ts @@ -1,8 +1,8 @@ +import { pathToArray, relocatedError } from '@graphql-tools/utils'; import { handleMaybePromise } from '@whatwg-node/promise-helpers'; import { GraphQLError } from 'graphql'; import { getLoader } from './getLoader.js'; import { BatchDelegateOptions } from './types.js'; -import { pathToArray, relocatedError } from '@graphql-tools/utils'; export function batchDelegateToSchema( options: BatchDelegateOptions, @@ -16,12 +16,12 @@ export function batchDelegateToSchema( const loader = getLoader(options); return handleMaybePromise( () => (Array.isArray(key) ? loader.loadMany(key) : loader.load(key)), - res => res, - error => { + (res) => res, + (error) => { if (options.info?.path && error instanceof GraphQLError) { return relocatedError(error, pathToArray(options.info.path)); } return error; - } + }, ); } diff --git a/packages/batch-delegate/tests/errorPaths.test.ts b/packages/batch-delegate/tests/errorPaths.test.ts index 189c86b38..15fd80f51 100644 --- a/packages/batch-delegate/tests/errorPaths.test.ts +++ b/packages/batch-delegate/tests/errorPaths.test.ts @@ -1,30 +1,30 @@ -import { GraphQLError, parse } from 'graphql'; import { batchDelegateToSchema } from '@graphql-tools/batch-delegate'; import { delegateToSchema } from '@graphql-tools/delegate'; +import { normalizedExecutor } from '@graphql-tools/executor'; import { makeExecutableSchema } from '@graphql-tools/schema'; import { stitchSchemas } from '@graphql-tools/stitch'; +import { GraphQLError, parse } from 'graphql'; import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { normalizedExecutor } from '@graphql-tools/executor'; class NotFoundError extends GraphQLError { - constructor(id: unknown) { - super('Not Found', { - extensions: { id }, - }) - } + constructor(id: unknown) { + super('Not Found', { + extensions: { id }, + }); + } } describe('preserves error path indices', () => { - const getProperty = vi.fn((id: unknown) => { - return new NotFoundError(id); - }); + const getProperty = vi.fn((id: unknown) => { + return new NotFoundError(id); + }); - beforeEach(() => { - getProperty.mockClear(); - }); + beforeEach(() => { + getProperty.mockClear(); + }); - const subschema = makeExecutableSchema({ - typeDefs: /* GraphQL */ ` + const subschema = makeExecutableSchema({ + typeDefs: /* GraphQL */ ` type Property { id: ID! } @@ -40,28 +40,28 @@ describe('preserves error path indices', () => { propertiesByIds(ids: [ID!]!): [Property]! } `, - resolvers: { - Query: { - objects: () => { - return [ - { id: '1', propertyId: '1' }, - { id: '2', propertyId: '1' }, - ]; - }, - propertyById: (_, args) => getProperty(args.id), - propertiesByIds: (_, args) => args.ids.map(getProperty), - }, + resolvers: { + Query: { + objects: () => { + return [ + { id: '1', propertyId: '1' }, + { id: '2', propertyId: '1' }, + ]; }, - }); - - const subschemas = [subschema]; - const typeDefs = /* GraphQL */ ` + propertyById: (_, args) => getProperty(args.id), + propertiesByIds: (_, args) => args.ids.map(getProperty), + }, + }, + }); + + const subschemas = [subschema]; + const typeDefs = /* GraphQL */ ` extend type Object { property: Property } `; - const query = /* GraphQL */ ` + const query = /* GraphQL */ ` query { objects { id @@ -72,91 +72,91 @@ describe('preserves error path indices', () => { } `; - const expected = { - errors: [ - { - message: 'Not Found', - extensions: { id: '1' }, - path: ['objects', 0, 'property'], - }, - { - message: 'Not Found', - extensions: { id: '1' }, - path: ['objects', 1, 'property'], - }, - ], - data: { - objects: [ - { - id: '1', - property: null, - }, - { - id: '2', - property: null, - }, - ], + const expected = { + errors: [ + { + message: 'Not Found', + extensions: { id: '1' }, + path: ['objects', 0, 'property'], + }, + { + message: 'Not Found', + extensions: { id: '1' }, + path: ['objects', 1, 'property'], + }, + ], + data: { + objects: [ + { + id: '1', + property: null as null, }, - }; - - test('using delegateToSchema', async () => { - const schema = stitchSchemas({ - subschemas, - typeDefs, - resolvers: { - Object: { - property: { - selectionSet: '{ propertyId }', - resolve: (source, _, context, info) => { - return delegateToSchema({ - schema: subschema, - fieldName: 'propertyById', - args: { id: source.propertyId }, - context, - info, - }); - }, - }, - }, + { + id: '2', + property: null as null, + }, + ], + }, + }; + + test('using delegateToSchema', async () => { + const schema = stitchSchemas({ + subschemas, + typeDefs, + resolvers: { + Object: { + property: { + selectionSet: '{ propertyId }', + resolve: (source, _, context, info) => { + return delegateToSchema({ + schema: subschema, + fieldName: 'propertyById', + args: { id: source.propertyId }, + context, + info, + }); }, - }); - - - const result = await normalizedExecutor({ - schema, - document: parse(query), - }) - - expect(getProperty).toBeCalledTimes(2); - expect(result).toMatchObject(expected); + }, + }, + }, }); - test('using batchDelegateToSchema', async () => { - const schema = stitchSchemas({ - subschemas, - typeDefs, - resolvers: { - Object: { - property: { - selectionSet: '{ propertyId }', - resolve: (source, _, context, info) => batchDelegateToSchema({ - schema: subschema, - fieldName: 'propertiesByIds', - key: source.propertyId, - context, - info, - }), - }, - }, - }, - }); + const result = await normalizedExecutor({ + schema, + document: parse(query), + }); - const result = await normalizedExecutor({ - schema, - document: parse(query), - }) + expect(getProperty).toBeCalledTimes(2); + expect(result).toMatchObject(expected); + }); + + test('using batchDelegateToSchema', async () => { + const schema = stitchSchemas({ + subschemas, + typeDefs, + resolvers: { + Object: { + property: { + selectionSet: '{ propertyId }', + resolve: (source, _, context, info) => + batchDelegateToSchema({ + schema: subschema, + fieldName: 'propertiesByIds', + key: source.propertyId, + context, + info, + }), + }, + }, + }, + }); - expect(getProperty).toBeCalledTimes(1); - expect(result).toMatchObject(expected); + const result = await normalizedExecutor({ + schema, + document: parse(query), }); -}); \ No newline at end of file + + expect(getProperty).toBeCalledTimes(1); + expect(result).toMatchObject(expected); + }); +}); From b2f1a61f20f7909efe55c2d1a8998c3d5d778b9d Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 24 Sep 2025 17:48:32 +0300 Subject: [PATCH 4/5] No need for handleMaybePromise --- .../src/batchDelegateToSchema.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/batch-delegate/src/batchDelegateToSchema.ts b/packages/batch-delegate/src/batchDelegateToSchema.ts index 1c882ea4c..50807e2b9 100644 --- a/packages/batch-delegate/src/batchDelegateToSchema.ts +++ b/packages/batch-delegate/src/batchDelegateToSchema.ts @@ -1,5 +1,4 @@ import { pathToArray, relocatedError } from '@graphql-tools/utils'; -import { handleMaybePromise } from '@whatwg-node/promise-helpers'; import { GraphQLError } from 'graphql'; import { getLoader } from './getLoader.js'; import { BatchDelegateOptions } from './types.js'; @@ -14,14 +13,11 @@ export function batchDelegateToSchema( return []; } const loader = getLoader(options); - return handleMaybePromise( - () => (Array.isArray(key) ? loader.loadMany(key) : loader.load(key)), - (res) => res, - (error) => { - if (options.info?.path && error instanceof GraphQLError) { - return relocatedError(error, pathToArray(options.info.path)); - } - return error; - }, - ); + const res = Array.isArray(key) ? loader.loadMany(key) : loader.load(key); + return res.catch((error) => { + if (options.info?.path && error instanceof GraphQLError) { + return relocatedError(error, pathToArray(options.info.path)); + } + return error; + }); } From 14e4b0ce773be2d357065c1e138a2b8366018c51 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Wed, 24 Sep 2025 19:26:03 +0300 Subject: [PATCH 5/5] Fix jest --- packages/batch-delegate/tests/errorPaths.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/batch-delegate/tests/errorPaths.test.ts b/packages/batch-delegate/tests/errorPaths.test.ts index 15fd80f51..78f065ce5 100644 --- a/packages/batch-delegate/tests/errorPaths.test.ts +++ b/packages/batch-delegate/tests/errorPaths.test.ts @@ -126,7 +126,7 @@ describe('preserves error path indices', () => { document: parse(query), }); - expect(getProperty).toBeCalledTimes(2); + expect(getProperty).toHaveBeenCalledTimes(2); expect(result).toMatchObject(expected); }); @@ -156,7 +156,7 @@ describe('preserves error path indices', () => { document: parse(query), }); - expect(getProperty).toBeCalledTimes(1); + expect(getProperty).toHaveBeenCalledTimes(1); expect(result).toMatchObject(expected); }); });