Skip to content

Commit 4ed99d5

Browse files
yaacovCRyaacovCR
committed
including mixture of sync/async errors in lists
following graphql#3706 fix for field execution
1 parent 7a609a2 commit 4ed99d5

File tree

2 files changed

+216
-94
lines changed

2 files changed

+216
-94
lines changed

‎src/execution/__tests__/lists-test.ts

+98-9
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,17 @@ describe('Execute: Accepts async iterables as list value', () => {
8989

9090
function completeObjectList(
9191
resolve: GraphQLFieldResolver<{ index: number }, unknown>,
92+
nonNullable = false,
9293
): PromiseOrValue<ExecutionResult> {
94+
const ObjectWrapperType = new GraphQLObjectType({
95+
name: 'ObjectWrapper',
96+
fields: {
97+
index: {
98+
type: new GraphQLNonNull(GraphQLString),
99+
resolve,
100+
},
101+
},
102+
});
93103
const schema = new GraphQLSchema({
94104
query: new GraphQLObjectType({
95105
name: 'Query',
@@ -101,15 +111,9 @@ describe('Execute: Accepts async iterables as list value', () => {
101111
yield await Promise.resolve({ index: 2 });
102112
},
103113
type: new GraphQLList(
104-
new GraphQLObjectType({
105-
name: 'ObjectWrapper',
106-
fields: {
107-
index: {
108-
type: new GraphQLNonNull(GraphQLString),
109-
resolve,
110-
},
111-
},
112-
}),
114+
nonNullable
115+
? new GraphQLNonNull(ObjectWrapperType)
116+
: ObjectWrapperType,
113117
),
114118
},
115119
},
@@ -216,6 +220,27 @@ describe('Execute: Accepts async iterables as list value', () => {
216220
],
217221
});
218222
});
223+
224+
it('Handles mixture of synchronous and asynchronous errors from `completeValue` in AsyncIterables', async () => {
225+
expectJSON(
226+
await completeObjectList(({ index }) => {
227+
if (index === 0) {
228+
return Promise.reject(new Error('bad'));
229+
}
230+
throw new Error('also bad');
231+
}, true),
232+
).toDeepEqual({
233+
data: { listField: null },
234+
errors: [
235+
{
236+
message: 'also bad',
237+
locations: [{ line: 1, column: 15 }],
238+
path: ['listField', 1, 'index'],
239+
},
240+
],
241+
});
242+
});
243+
219244
it('Handles nulls yielded by async generator', async () => {
220245
async function* listField() {
221246
yield await Promise.resolve(1);
@@ -265,6 +290,11 @@ describe('Execute: Handles list nullability', () => {
265290
expectJSON(await executeQuery(promisify(listOfPromises))).toDeepEqual(
266291
result,
267292
);
293+
294+
// Test mix of synchronous and non-synchronous values
295+
const [first, ...rest] = listField;
296+
const listOfSomePromises = [first, ...rest.map(promisify)];
297+
expectJSON(await executeQuery(listOfSomePromises)).toDeepEqual(result);
268298
}
269299
return result;
270300

@@ -322,6 +352,32 @@ describe('Execute: Handles list nullability', () => {
322352
});
323353
});
324354

355+
it('Contains multiple nulls', async () => {
356+
const listField = [null, null, 2];
357+
const errors = [
358+
{
359+
message: 'Cannot return null for non-nullable field Query.listField.',
360+
locations: [{ line: 1, column: 3 }],
361+
path: ['listField', 0],
362+
},
363+
];
364+
365+
expect(await complete({ listField, as: '[Int]' })).to.deep.equal({
366+
data: { listField: [null, null, 2] },
367+
});
368+
expect(await complete({ listField, as: '[Int]!' })).to.deep.equal({
369+
data: { listField: [null, null, 2] },
370+
});
371+
expectJSON(await complete({ listField, as: '[Int!]' })).toDeepEqual({
372+
data: { listField: null },
373+
errors,
374+
});
375+
expectJSON(await complete({ listField, as: '[Int!]!' })).toDeepEqual({
376+
data: null,
377+
errors,
378+
});
379+
});
380+
325381
it('Returns null', async () => {
326382
const listField = null;
327383
const errors = [
@@ -376,6 +432,39 @@ describe('Execute: Handles list nullability', () => {
376432
});
377433
});
378434

435+
it('Contains multiple errors', async () => {
436+
const listField = [new Error('bad'), new Error('also bad'), 2];
437+
438+
const firstError = {
439+
message: 'bad',
440+
locations: [{ line: 1, column: 3 }],
441+
path: ['listField', 0],
442+
};
443+
444+
const secondError = {
445+
message: 'also bad',
446+
locations: [{ line: 1, column: 3 }],
447+
path: ['listField', 1],
448+
};
449+
450+
expectJSON(await complete({ listField, as: '[Int]' })).toDeepEqual({
451+
data: { listField: [null, null, 2] },
452+
errors: [firstError, secondError],
453+
});
454+
expectJSON(await complete({ listField, as: '[Int]!' })).toDeepEqual({
455+
data: { listField: [null, null, 2] },
456+
errors: [firstError, secondError],
457+
});
458+
expectJSON(await complete({ listField, as: '[Int!]' })).toDeepEqual({
459+
data: { listField: null },
460+
errors: [firstError],
461+
});
462+
expectJSON(await complete({ listField, as: '[Int!]!' })).toDeepEqual({
463+
data: null,
464+
errors: [firstError],
465+
});
466+
});
467+
379468
it('Results in error', async () => {
380469
const listField = new Error('bad');
381470
const errors = [

‎src/execution/execute.ts

+118-85
Original file line numberDiff line numberDiff line change
@@ -1029,59 +1029,69 @@ async function completeAsyncIteratorValue(
10291029
let containsPromise = false;
10301030
const completedResults: Array<unknown> = [];
10311031
let index = 0;
1032-
// eslint-disable-next-line no-constant-condition
1033-
while (true) {
1034-
if (
1035-
stream &&
1036-
typeof stream.initialCount === 'number' &&
1037-
index >= stream.initialCount
1038-
) {
1039-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1040-
executeStreamIterator(
1041-
index,
1042-
iterator,
1043-
exeContext,
1044-
fieldNodes,
1045-
info,
1046-
itemType,
1047-
path,
1048-
stream.label,
1049-
asyncPayloadRecord,
1050-
);
1051-
break;
1052-
}
1032+
try {
1033+
// eslint-disable-next-line no-constant-condition
1034+
while (true) {
1035+
if (
1036+
stream &&
1037+
typeof stream.initialCount === 'number' &&
1038+
index >= stream.initialCount
1039+
) {
1040+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1041+
executeStreamIterator(
1042+
index,
1043+
iterator,
1044+
exeContext,
1045+
fieldNodes,
1046+
info,
1047+
itemType,
1048+
path,
1049+
stream.label,
1050+
asyncPayloadRecord,
1051+
);
1052+
break;
1053+
}
10531054

1054-
const itemPath = addPath(path, index, undefined);
1055-
let iteration;
1056-
try {
1057-
// eslint-disable-next-line no-await-in-loop
1058-
iteration = await iterator.next();
1059-
if (iteration.done) {
1055+
const itemPath = addPath(path, index, undefined);
1056+
let iteration;
1057+
try {
1058+
// eslint-disable-next-line no-await-in-loop
1059+
iteration = await iterator.next();
1060+
if (iteration.done) {
1061+
break;
1062+
}
1063+
} catch (rawError) {
1064+
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
1065+
completedResults.push(handleFieldError(error, itemType, errors));
10601066
break;
10611067
}
1062-
} catch (rawError) {
1063-
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
1064-
completedResults.push(handleFieldError(error, itemType, errors));
1065-
break;
1066-
}
10671068

1068-
if (
1069-
completeListItemValue(
1070-
iteration.value,
1071-
completedResults,
1072-
errors,
1073-
exeContext,
1074-
itemType,
1075-
fieldNodes,
1076-
info,
1077-
itemPath,
1078-
asyncPayloadRecord,
1079-
)
1080-
) {
1081-
containsPromise = true;
1069+
if (
1070+
completeListItemValue(
1071+
iteration.value,
1072+
completedResults,
1073+
errors,
1074+
exeContext,
1075+
itemType,
1076+
fieldNodes,
1077+
info,
1078+
itemPath,
1079+
asyncPayloadRecord,
1080+
)
1081+
) {
1082+
containsPromise = true;
1083+
}
1084+
index += 1;
10821085
}
1083-
index += 1;
1086+
} catch (error) {
1087+
if (containsPromise) {
1088+
return Promise.all(completedResults).finally(() => {
1089+
throw error;
1090+
});
1091+
}
1092+
throw error;
10841093
}
1094+
10851095
return containsPromise ? Promise.all(completedResults) : completedResults;
10861096
}
10871097

@@ -1129,48 +1139,71 @@ function completeListValue(
11291139
let previousAsyncPayloadRecord = asyncPayloadRecord;
11301140
const completedResults: Array<unknown> = [];
11311141
let index = 0;
1132-
for (const item of result) {
1133-
// No need to modify the info object containing the path,
1134-
// since from here on it is not ever accessed by resolver functions.
1135-
const itemPath = addPath(path, index, undefined);
1142+
const iterator = result[Symbol.iterator]();
1143+
try {
1144+
let iteration = iterator.next();
1145+
while (!iteration.done) {
1146+
const item = iteration.value;
1147+
// No need to modify the info object containing the path,
1148+
// since from here on it is not ever accessed by resolver functions.
1149+
const itemPath = addPath(path, index, undefined);
1150+
1151+
if (
1152+
stream &&
1153+
typeof stream.initialCount === 'number' &&
1154+
index >= stream.initialCount
1155+
) {
1156+
previousAsyncPayloadRecord = executeStreamField(
1157+
path,
1158+
itemPath,
1159+
item,
1160+
exeContext,
1161+
fieldNodes,
1162+
info,
1163+
itemType,
1164+
stream.label,
1165+
previousAsyncPayloadRecord,
1166+
);
1167+
index++;
1168+
iteration = iterator.next();
1169+
continue;
1170+
}
1171+
1172+
if (
1173+
completeListItemValue(
1174+
item,
1175+
completedResults,
1176+
errors,
1177+
exeContext,
1178+
itemType,
1179+
fieldNodes,
1180+
info,
1181+
itemPath,
1182+
asyncPayloadRecord,
1183+
)
1184+
) {
1185+
containsPromise = true;
1186+
}
11361187

1137-
if (
1138-
stream &&
1139-
typeof stream.initialCount === 'number' &&
1140-
index >= stream.initialCount
1141-
) {
1142-
previousAsyncPayloadRecord = executeStreamField(
1143-
path,
1144-
itemPath,
1145-
item,
1146-
exeContext,
1147-
fieldNodes,
1148-
info,
1149-
itemType,
1150-
stream.label,
1151-
previousAsyncPayloadRecord,
1152-
);
11531188
index++;
1154-
continue;
1189+
iteration = iterator.next();
11551190
}
1156-
1157-
if (
1158-
completeListItemValue(
1159-
item,
1160-
completedResults,
1161-
errors,
1162-
exeContext,
1163-
itemType,
1164-
fieldNodes,
1165-
info,
1166-
itemPath,
1167-
asyncPayloadRecord,
1168-
)
1169-
) {
1170-
containsPromise = true;
1191+
} catch (error) {
1192+
let iteration = iterator.next();
1193+
while (!iteration.done) {
1194+
const item = iteration.value;
1195+
if (isPromise(item)) {
1196+
containsPromise = true;
1197+
completedResults.push(item);
1198+
}
1199+
iteration = iterator.next();
11711200
}
1172-
1173-
index++;
1201+
if (containsPromise) {
1202+
return Promise.all(completedResults).finally(() => {
1203+
throw error;
1204+
});
1205+
}
1206+
throw error;
11741207
}
11751208

11761209
return containsPromise ? Promise.all(completedResults) : completedResults;

0 commit comments

Comments
 (0)