Skip to content

Commit 058ef2f

Browse files
ardatanCopilot
andauthored
fix(batch-delegate): correct error paths in case of batch delegation (#1556)
Co-authored-by: Copilot <[email protected]>
1 parent 19a6cc4 commit 058ef2f

File tree

4 files changed

+179
-3
lines changed

4 files changed

+179
-3
lines changed

.changeset/breezy-weeks-camp.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@graphql-tools/batch-delegate': patch
3+
'@graphql-tools/delegate': patch
4+
---
5+
6+
Correct error paths in case of batch delegation with the same error

packages/batch-delegate/src/batchDelegateToSchema.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { pathToArray, relocatedError } from '@graphql-tools/utils';
2+
import { GraphQLError } from 'graphql';
13
import { getLoader } from './getLoader.js';
24
import { BatchDelegateOptions } from './types.js';
35

@@ -11,5 +13,11 @@ export function batchDelegateToSchema<TContext = any, K = any, V = any, C = K>(
1113
return [];
1214
}
1315
const loader = getLoader(options);
14-
return Array.isArray(key) ? loader.loadMany(key) : loader.load(key);
16+
const res = Array.isArray(key) ? loader.loadMany(key) : loader.load(key);
17+
return res.catch((error) => {
18+
if (options.info?.path && error instanceof GraphQLError) {
19+
return relocatedError(error, pathToArray(options.info.path));
20+
}
21+
return error;
22+
});
1523
}
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import { batchDelegateToSchema } from '@graphql-tools/batch-delegate';
2+
import { delegateToSchema } from '@graphql-tools/delegate';
3+
import { normalizedExecutor } from '@graphql-tools/executor';
4+
import { makeExecutableSchema } from '@graphql-tools/schema';
5+
import { stitchSchemas } from '@graphql-tools/stitch';
6+
import { GraphQLError, parse } from 'graphql';
7+
import { beforeEach, describe, expect, test, vi } from 'vitest';
8+
9+
class NotFoundError extends GraphQLError {
10+
constructor(id: unknown) {
11+
super('Not Found', {
12+
extensions: { id },
13+
});
14+
}
15+
}
16+
17+
describe('preserves error path indices', () => {
18+
const getProperty = vi.fn((id: unknown) => {
19+
return new NotFoundError(id);
20+
});
21+
22+
beforeEach(() => {
23+
getProperty.mockClear();
24+
});
25+
26+
const subschema = makeExecutableSchema({
27+
typeDefs: /* GraphQL */ `
28+
type Property {
29+
id: ID!
30+
}
31+
32+
type Object {
33+
id: ID!
34+
propertyId: ID!
35+
}
36+
37+
type Query {
38+
objects: [Object!]!
39+
propertyById(id: ID!): Property
40+
propertiesByIds(ids: [ID!]!): [Property]!
41+
}
42+
`,
43+
resolvers: {
44+
Query: {
45+
objects: () => {
46+
return [
47+
{ id: '1', propertyId: '1' },
48+
{ id: '2', propertyId: '1' },
49+
];
50+
},
51+
propertyById: (_, args) => getProperty(args.id),
52+
propertiesByIds: (_, args) => args.ids.map(getProperty),
53+
},
54+
},
55+
});
56+
57+
const subschemas = [subschema];
58+
const typeDefs = /* GraphQL */ `
59+
extend type Object {
60+
property: Property
61+
}
62+
`;
63+
64+
const query = /* GraphQL */ `
65+
query {
66+
objects {
67+
id
68+
property {
69+
id
70+
}
71+
}
72+
}
73+
`;
74+
75+
const expected = {
76+
errors: [
77+
{
78+
message: 'Not Found',
79+
extensions: { id: '1' },
80+
path: ['objects', 0, 'property'],
81+
},
82+
{
83+
message: 'Not Found',
84+
extensions: { id: '1' },
85+
path: ['objects', 1, 'property'],
86+
},
87+
],
88+
data: {
89+
objects: [
90+
{
91+
id: '1',
92+
property: null as null,
93+
},
94+
{
95+
id: '2',
96+
property: null as null,
97+
},
98+
],
99+
},
100+
};
101+
102+
test('using delegateToSchema', async () => {
103+
const schema = stitchSchemas({
104+
subschemas,
105+
typeDefs,
106+
resolvers: {
107+
Object: {
108+
property: {
109+
selectionSet: '{ propertyId }',
110+
resolve: (source, _, context, info) => {
111+
return delegateToSchema({
112+
schema: subschema,
113+
fieldName: 'propertyById',
114+
args: { id: source.propertyId },
115+
context,
116+
info,
117+
});
118+
},
119+
},
120+
},
121+
},
122+
});
123+
124+
const result = await normalizedExecutor({
125+
schema,
126+
document: parse(query),
127+
});
128+
129+
expect(getProperty).toHaveBeenCalledTimes(2);
130+
expect(result).toMatchObject(expected);
131+
});
132+
133+
test('using batchDelegateToSchema', async () => {
134+
const schema = stitchSchemas({
135+
subschemas,
136+
typeDefs,
137+
resolvers: {
138+
Object: {
139+
property: {
140+
selectionSet: '{ propertyId }',
141+
resolve: (source, _, context, info) =>
142+
batchDelegateToSchema({
143+
schema: subschema,
144+
fieldName: 'propertiesByIds',
145+
key: source.propertyId,
146+
context,
147+
info,
148+
}),
149+
},
150+
},
151+
},
152+
});
153+
154+
const result = await normalizedExecutor({
155+
schema,
156+
document: parse(query),
157+
});
158+
159+
expect(getProperty).toHaveBeenCalledTimes(1);
160+
expect(result).toMatchObject(expected);
161+
});
162+
});

packages/delegate/src/resolveExternalValue.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,15 +182,15 @@ function resolveExternalList<TContext extends Record<string, any>>(
182182
);
183183
}
184184

185-
const reportedErrors = new WeakMap<GraphQLError, boolean>();
185+
const reportedErrors = new WeakSet<GraphQLError>();
186186

187187
function reportUnpathedErrorsViaNull(unpathedErrors: Array<GraphQLError>) {
188188
if (unpathedErrors.length) {
189189
const unreportedErrors: Array<GraphQLError> = [];
190190
for (const error of unpathedErrors) {
191191
if (!reportedErrors.has(error)) {
192192
unreportedErrors.push(error);
193-
reportedErrors.set(error, true);
193+
reportedErrors.add(error);
194194
}
195195
}
196196

0 commit comments

Comments
 (0)