Skip to content

Commit

Permalink
use complete helpers within executeField
Browse files Browse the repository at this point in the history
The optimized helpers change the order of promise resolution and affect
the value of hasNext.
  • Loading branch information
yaacovCR committed Oct 12, 2022
1 parent e5d982f commit 68b229e
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 64 deletions.
30 changes: 15 additions & 15 deletions src/execution/__tests__/nonnull-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,16 @@ describe('Execute: handles non-nullable types', () => {
path: ['syncNest', 'syncNest', 'sync'],
locations: [{ line: 6, column: 22 }],
},
{
message: promiseError.message,
path: ['syncNest', 'promise'],
locations: [{ line: 5, column: 11 }],
},
{
message: promiseError.message,
path: ['syncNest', 'syncNest', 'promise'],
locations: [{ line: 6, column: 27 }],
},
{
message: syncError.message,
path: ['syncNest', 'promiseNest', 'sync'],
Expand All @@ -274,21 +284,6 @@ describe('Execute: handles non-nullable types', () => {
path: ['promiseNest', 'syncNest', 'sync'],
locations: [{ line: 12, column: 22 }],
},
{
message: promiseError.message,
path: ['syncNest', 'promise'],
locations: [{ line: 5, column: 11 }],
},
{
message: promiseError.message,
path: ['syncNest', 'syncNest', 'promise'],
locations: [{ line: 6, column: 27 }],
},
{
message: syncError.message,
path: ['promiseNest', 'promiseNest', 'sync'],
locations: [{ line: 13, column: 25 }],
},
{
message: promiseError.message,
path: ['syncNest', 'promiseNest', 'promise'],
Expand All @@ -304,6 +299,11 @@ describe('Execute: handles non-nullable types', () => {
path: ['promiseNest', 'syncNest', 'promise'],
locations: [{ line: 12, column: 27 }],
},
{
message: syncError.message,
path: ['promiseNest', 'promiseNest', 'sync'],
locations: [{ line: 13, column: 25 }],
},
{
message: promiseError.message,
path: ['promiseNest', 'promiseNest', 'promise'],
Expand Down
34 changes: 23 additions & 11 deletions src/execution/__tests__/stream-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,9 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand All @@ -1060,19 +1063,25 @@ describe('Execute: stream directive', () => {
} /* c8 ignore stop */,
},
});
expectJSON(result).toDeepEqual({
errors: [
{
message:
'Cannot return null for non-nullable field NestedObject.nonNullScalarField.',
locations: [{ line: 4, column: 11 }],
path: ['nestedObject', 'nonNullScalarField'],
expectJSON(result).toDeepEqual([
{
errors: [
{
message:
'Cannot return null for non-nullable field NestedObject.nonNullScalarField.',
locations: [{ line: 4, column: 11 }],
path: ['nestedObject', 'nonNullScalarField'],
},
],
data: {
nestedObject: null,
},
],
data: {
nestedObject: null,
hasNext: true,
},
});
{
hasNext: false,
},
]);
});
it('Filters payloads that are nulled by a later synchronous error', async () => {
const document = parse(`
Expand Down Expand Up @@ -1213,6 +1222,9 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand Down
62 changes: 24 additions & 38 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ function executeField(
);

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

const result = resolveFn(source, args, contextValue, info);

let completed;
if (isPromise(result)) {
completed = result.then((resolved) =>
completeValue(
exeContext,
returnType,
fieldNodes,
info,
path,
resolved,
asyncPayloadRecord,
),
);
} else {
completed = completeValue(
exeContext,
returnType,
fieldNodes,
info,
path,
result,
asyncPayloadRecord,
);
}

if (isPromise(completed)) {
// Note: we don't rely on a `catch` method, but we do expect "thenable"
// to take a second callback for the error case.
return completed.then(undefined, (rawError) => {
const error = locatedError(rawError, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return handledError;
});
}
return completed;
result = resolveFn(source, args, contextValue, info);
} catch (rawError) {
const error = locatedError(rawError, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return handledError;
}

if (isPromise(result)) {
return completePromiseCatchingErrors(
exeContext,
returnType,
fieldNodes,
info,
path,
result,
asyncPayloadRecord,
);
}

return completeValueCatchingErrors(
exeContext,
returnType,
fieldNodes,
info,
path,
result,
asyncPayloadRecord,
);
}

/**
Expand Down

0 comments on commit 68b229e

Please sign in to comment.