Skip to content

Commit

Permalink
introduce completePromiseCatchingErrors helpers
Browse files Browse the repository at this point in the history
use it within executeField, completeListItemValue, executeStreamField

this takes advantage of some quirks involving promises/await
  • Loading branch information
yaacovCR committed Dec 13, 2022
1 parent 500b458 commit 2030515
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 88 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 @@ -1174,6 +1174,9 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand All @@ -1197,19 +1200,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 @@ -1350,6 +1359,9 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand Down
124 changes: 62 additions & 62 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -715,25 +715,15 @@ function executeField(
const result = resolveFn(source, args, contextValue, info);

if (isPromise(result)) {
const completed = result.then((resolved) =>
completeValue(
exeContext,
returnType,
fieldNodes,
info,
path,
resolved,
asyncPayloadRecord,
),
return completePromise(
exeContext,
returnType,
fieldNodes,
info,
path,
result,
asyncPayloadRecord,
);
// 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;
});
}

const completed = completeValue(
Expand Down Expand Up @@ -922,6 +912,41 @@ function completeValue(
);
}

async function completePromise(
exeContext: ExecutionContext,
returnType: GraphQLOutputType,
fieldNodes: ReadonlyArray<FieldNode>,
info: GraphQLResolveInfo,
path: Path,
result: Promise<unknown>,
asyncPayloadRecord?: AsyncPayloadRecord,
): Promise<unknown> {
try {
const resolved = await result;
let completed = completeValue(
exeContext,
returnType,
fieldNodes,
info,
path,
resolved,
asyncPayloadRecord,
);
if (isPromise(completed)) {
// see: https:/tc39/proposal-faster-promise-adoption
// it is faster to await a promise prior to returning it from an async function
completed = await completed;
}
return completed;
} catch (rawError) {
const errors = asyncPayloadRecord?.errors ?? exeContext.errors;
const error = locatedError(rawError, fieldNodes, pathToArray(path));
const handledError = handleFieldError(error, returnType, errors);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return handledError;
}
}

/**
* Returns an object containing the `@stream` arguments if a field should be
* streamed based on the experimental flag, stream directive present and
Expand Down Expand Up @@ -1156,29 +1181,18 @@ function completeListItemValue(
asyncPayloadRecord?: AsyncPayloadRecord,
): boolean {
if (isPromise(item)) {
const completedItem = item.then((resolved) =>
completeValue(
completedResults.push(
completePromise(
exeContext,
itemType,
fieldNodes,
info,
itemPath,
resolved,
item,
asyncPayloadRecord,
),
);

// Note: we don't rely on a `catch` method, but we do expect "thenable"
// to take a second callback for the error case.
completedResults.push(
completedItem.then(undefined, (rawError) => {
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
const handledError = handleFieldError(error, itemType, errors);
filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord);
return handledError;
}),
);

return true;
}

Expand Down Expand Up @@ -1897,36 +1911,22 @@ function executeStreamField(
exeContext,
});
if (isPromise(item)) {
const completedItems = item
.then((resolved) =>
completeValue(
exeContext,
itemType,
fieldNodes,
info,
itemPath,
resolved,
asyncPayloadRecord,
),
)
.then(undefined, (rawError) => {
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
const handledError = handleFieldError(
error,
itemType,
asyncPayloadRecord.errors,
);
filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord);
return handledError;
})
.then(
(value) => [value],
(error) => {
asyncPayloadRecord.errors.push(error);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return null;
},
);
const completedItems = completePromise(
exeContext,
itemType,
fieldNodes,
info,
itemPath,
item,
asyncPayloadRecord,
).then(
(value) => [value],
(error) => {
asyncPayloadRecord.errors.push(error);
filterSubsequentPayloads(exeContext, path, asyncPayloadRecord);
return null;
},
);

asyncPayloadRecord.addItems(completedItems);
return asyncPayloadRecord;
Expand Down

0 comments on commit 2030515

Please sign in to comment.