Skip to content

Commit 68b229e

Browse files
committed
use complete helpers within executeField
The optimized helpers change the order of promise resolution and affect the value of hasNext.
1 parent e5d982f commit 68b229e

File tree

3 files changed

+62
-64
lines changed

3 files changed

+62
-64
lines changed

src/execution/__tests__/nonnull-test.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,16 @@ describe('Execute: handles non-nullable types', () => {
259259
path: ['syncNest', 'syncNest', 'sync'],
260260
locations: [{ line: 6, column: 22 }],
261261
},
262+
{
263+
message: promiseError.message,
264+
path: ['syncNest', 'promise'],
265+
locations: [{ line: 5, column: 11 }],
266+
},
267+
{
268+
message: promiseError.message,
269+
path: ['syncNest', 'syncNest', 'promise'],
270+
locations: [{ line: 6, column: 27 }],
271+
},
262272
{
263273
message: syncError.message,
264274
path: ['syncNest', 'promiseNest', 'sync'],
@@ -274,21 +284,6 @@ describe('Execute: handles non-nullable types', () => {
274284
path: ['promiseNest', 'syncNest', 'sync'],
275285
locations: [{ line: 12, column: 22 }],
276286
},
277-
{
278-
message: promiseError.message,
279-
path: ['syncNest', 'promise'],
280-
locations: [{ line: 5, column: 11 }],
281-
},
282-
{
283-
message: promiseError.message,
284-
path: ['syncNest', 'syncNest', 'promise'],
285-
locations: [{ line: 6, column: 27 }],
286-
},
287-
{
288-
message: syncError.message,
289-
path: ['promiseNest', 'promiseNest', 'sync'],
290-
locations: [{ line: 13, column: 25 }],
291-
},
292287
{
293288
message: promiseError.message,
294289
path: ['syncNest', 'promiseNest', 'promise'],
@@ -304,6 +299,11 @@ describe('Execute: handles non-nullable types', () => {
304299
path: ['promiseNest', 'syncNest', 'promise'],
305300
locations: [{ line: 12, column: 27 }],
306301
},
302+
{
303+
message: syncError.message,
304+
path: ['promiseNest', 'promiseNest', 'sync'],
305+
locations: [{ line: 13, column: 25 }],
306+
},
307307
{
308308
message: promiseError.message,
309309
path: ['promiseNest', 'promiseNest', 'promise'],

src/execution/__tests__/stream-test.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,9 @@ describe('Execute: stream directive', () => {
10371037
],
10381038
},
10391039
],
1040+
hasNext: true,
1041+
},
1042+
{
10401043
hasNext: false,
10411044
},
10421045
]);
@@ -1060,19 +1063,25 @@ describe('Execute: stream directive', () => {
10601063
} /* c8 ignore stop */,
10611064
},
10621065
});
1063-
expectJSON(result).toDeepEqual({
1064-
errors: [
1065-
{
1066-
message:
1067-
'Cannot return null for non-nullable field NestedObject.nonNullScalarField.',
1068-
locations: [{ line: 4, column: 11 }],
1069-
path: ['nestedObject', 'nonNullScalarField'],
1066+
expectJSON(result).toDeepEqual([
1067+
{
1068+
errors: [
1069+
{
1070+
message:
1071+
'Cannot return null for non-nullable field NestedObject.nonNullScalarField.',
1072+
locations: [{ line: 4, column: 11 }],
1073+
path: ['nestedObject', 'nonNullScalarField'],
1074+
},
1075+
],
1076+
data: {
1077+
nestedObject: null,
10701078
},
1071-
],
1072-
data: {
1073-
nestedObject: null,
1079+
hasNext: true,
10741080
},
1075-
});
1081+
{
1082+
hasNext: false,
1083+
},
1084+
]);
10761085
});
10771086
it('Filters payloads that are nulled by a later synchronous error', async () => {
10781087
const document = parse(`
@@ -1213,6 +1222,9 @@ describe('Execute: stream directive', () => {
12131222
],
12141223
},
12151224
],
1225+
hasNext: true,
1226+
},
1227+
{
12161228
hasNext: false,
12171229
},
12181230
]);

src/execution/execute.ts

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,7 @@ function executeField(
687687
);
688688

689689
// Get the resolve function, regardless of if its result is normal or abrupt (error).
690+
let result: PromiseOrValue<unknown>;
690691
try {
691692
// Build a JS object of arguments from the field.arguments AST, using the
692693
// variables scope to fulfill any variable references.
@@ -702,50 +703,35 @@ function executeField(
702703
// used to represent an authenticated user, or request-specific caches.
703704
const contextValue = exeContext.contextValue;
704705

705-
const result = resolveFn(source, args, contextValue, info);
706-
707-
let completed;
708-
if (isPromise(result)) {
709-
completed = result.then((resolved) =>
710-
completeValue(
711-
exeContext,
712-
returnType,
713-
fieldNodes,
714-
info,
715-
path,
716-
resolved,
717-
asyncPayloadRecord,
718-
),
719-
);
720-
} else {
721-
completed = completeValue(
722-
exeContext,
723-
returnType,
724-
fieldNodes,
725-
info,
726-
path,
727-
result,
728-
asyncPayloadRecord,
729-
);
730-
}
731-
732-
if (isPromise(completed)) {
733-
// Note: we don't rely on a `catch` method, but we do expect "thenable"
734-
// to take a second callback for the error case.
735-
return completed.then(undefined, (rawError) => {
736-
const error = locatedError(rawError, fieldNodes, pathToArray(path));
737-
const handledError = handleFieldError(error, returnType, errors);
738-
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
739-
return handledError;
740-
});
741-
}
742-
return completed;
706+
result = resolveFn(source, args, contextValue, info);
743707
} catch (rawError) {
744708
const error = locatedError(rawError, fieldNodes, pathToArray(path));
745709
const handledError = handleFieldError(error, returnType, errors);
746710
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
747711
return handledError;
748712
}
713+
714+
if (isPromise(result)) {
715+
return completePromiseCatchingErrors(
716+
exeContext,
717+
returnType,
718+
fieldNodes,
719+
info,
720+
path,
721+
result,
722+
asyncPayloadRecord,
723+
);
724+
}
725+
726+
return completeValueCatchingErrors(
727+
exeContext,
728+
returnType,
729+
fieldNodes,
730+
info,
731+
path,
732+
result,
733+
asyncPayloadRecord,
734+
);
749735
}
750736

751737
/**

0 commit comments

Comments
 (0)