diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index 6df435b1715..2b50d44583d 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -166,12 +166,13 @@ export interface FormattedCompletedResult { } export function buildIncrementalResponse( - result: GraphQLResult>, + result: ObjMap, errors: ReadonlyArray, + futures: ReadonlyArray, cancellableStreams: Set, ): ExperimentalIncrementalExecutionResults { const incrementalPublisher = new IncrementalPublisher(cancellableStreams); - return incrementalPublisher.buildResponse(result, errors); + return incrementalPublisher.buildResponse(result, errors, futures); } /** @@ -204,11 +205,10 @@ class IncrementalPublisher { } buildResponse( - result: GraphQLResult>, + data: ObjMap, errors: ReadonlyArray, + futures: ReadonlyArray, ): ExperimentalIncrementalExecutionResults { - const { result: data, futures } = result; - this._addFutures(futures); this._pruneEmpty(); @@ -636,20 +636,11 @@ export function isDeferredGroupedFieldSetRecord( return future instanceof DeferredGroupedFieldSetRecord; } -/** @internal **/ -export class GraphQLResult { - result: T; - futures: ReadonlyArray = []; - - constructor(result: T, futures: ReadonlyArray) { - this.result = result; - this.futures = futures; - } -} - export interface IncrementalContext { deferUsageSet: DeferUsageSet | undefined; errors: Array; + errorPaths: Set; + futures: Array; } export interface DeferredGroupedFieldSetResult { @@ -699,6 +690,8 @@ export class DeferredGroupedFieldSetRecord { const incrementalContext: IncrementalContext = { deferUsageSet, errors: [], + errorPaths: new Set(), + futures: [], }; for (const deferredFragmentRecord of this.deferredFragmentRecords) { @@ -793,6 +786,8 @@ export class StreamItemsRecord { const incrementalContext: IncrementalContext = { deferUsageSet: undefined, errors: [], + errorPaths: new Set(), + futures: [], }; this._result = executor(incrementalContext); diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index 4ae047aff08..03bf8126c6e 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/src/execution/__tests__/defer-test.ts @@ -1116,8 +1116,8 @@ describe('Execute: defer directive', () => { }, }, pending: [ - { id: '0', path: [] }, - { id: '1', path: ['a', 'b'] }, + { id: '0', path: ['a', 'b'] }, + { id: '1', path: [] }, ], hasNext: true, }, @@ -1125,14 +1125,14 @@ describe('Execute: defer directive', () => { incremental: [ { data: { e: { f: 'f' } }, - id: '1', + id: '0', }, { data: { g: { h: 'h' } }, - id: '0', + id: '1', }, ], - completed: [{ id: '1' }, { id: '0' }], + completed: [{ id: '0' }, { id: '1' }], hasNext: false, }, ]); diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts index 9c0a7ed22b6..22be4e4d354 100644 --- a/src/execution/__tests__/stream-test.ts +++ b/src/execution/__tests__/stream-test.ts @@ -2096,14 +2096,23 @@ describe('Execute: stream directive', () => { id: '2', }, ], - completed: [{ id: '2' }, { id: '1' }], - hasNext: false, + completed: [{ id: '2' }], + hasNext: true, }, done: false, }); const result5 = await iterator.next(); expectJSON(result5).toDeepEqual({ + value: { + completed: [{ id: '1' }], + hasNext: false, + }, + done: false, + }); + + const result6 = await iterator.next(); + expectJSON(result6).toDeepEqual({ value: undefined, done: true, }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 5082f3c94e5..78a0a2cc1c3 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -67,7 +67,7 @@ import { buildIncrementalResponse, DeferredFragmentRecord, DeferredGroupedFieldSetRecord, - GraphQLResult, + isDeferredGroupedFieldSetRecord, StreamItemsRecord, StreamRecord, } from './IncrementalPublisher.js'; @@ -148,6 +148,8 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; errors: Array; cancellableStreams: Set; + errorPaths: Set; + futures: Array; } export interface ExecutionArgs { @@ -256,32 +258,76 @@ function executeImpl( // Errors from sub-fields of a NonNull type may propagate to the top level, // at which point we still log the error and null the parent field, which // in this case is the entire response. - const { errors, cancellableStreams } = exeContext; + const { errors, errorPaths, futures, cancellableStreams } = exeContext; try { const data = executeOperation(exeContext); if (isPromise(data)) { return data.then( - (resolved) => buildDataResponse(resolved, errors, cancellableStreams), + (resolved) => + buildDataResponse( + resolved, + errors, + errorPaths, + futures, + cancellableStreams, + ), (error) => buildErrorResponse(error, errors), ); } - return buildDataResponse(data, exeContext.errors, cancellableStreams); + return buildDataResponse( + data, + exeContext.errors, + errorPaths, + futures, + cancellableStreams, + ); } catch (error) { return buildErrorResponse(error, exeContext.errors); } } function buildDataResponse( - data: ObjMap | GraphQLResult>, + data: ObjMap, errors: ReadonlyArray, + errorPaths: ReadonlySet, + futures: ReadonlyArray, cancellableStreams: Set, ): ExecutionResult | ExperimentalIncrementalExecutionResults { - if (data instanceof GraphQLResult) { - return buildIncrementalResponse(data, errors, cancellableStreams); + const filteredFutures = filterFutures(errorPaths, futures); + if (filteredFutures.length > 0) { + return buildIncrementalResponse(data, errors, futures, cancellableStreams); } return errors.length > 0 ? { errors, data } : { data }; } +function filterFutures( + errorPaths: ReadonlySet, + futures: ReadonlyArray, +): ReadonlyArray { + if (errorPaths.size === 0) { + return futures; + } + + const filteredFutures: Array = []; + for (const future of futures) { + let currentPath = isDeferredGroupedFieldSetRecord(future) + ? future.path + : future.streamRecord.path; + while (currentPath !== undefined) { + if (errorPaths.has(currentPath)) { + break; + } + currentPath = currentPath.prev; + } + if (errorPaths.has(currentPath)) { + continue; + } + filteredFutures.push(future); + } + + return filteredFutures; +} + function buildErrorResponse( error: GraphQLError, errors: Array, @@ -392,6 +438,8 @@ export function buildExecutionContext( typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, errors: [], + errorPaths: new Set(), + futures: [], cancellableStreams: new Set(), }; } @@ -412,8 +460,8 @@ function buildPerEventExecutionContext( */ function executeOperation( exeContext: ExecutionContext, -): PromiseOrValue | GraphQLResult>> { - const { operation, schema, fragments, variableValues, rootValue } = +): PromiseOrValue> { + const { operation, schema, fragments, variableValues, rootValue, futures } = exeContext; const rootType = schema.getRootType(operation.operation); if (rootType == null) { @@ -480,43 +528,12 @@ function executeOperation( newDeferMap, ); - return handleGraphQLResult(result, newDeferredGroupedFieldSetRecords); + futures.push(...newDeferredGroupedFieldSetRecords); } return result; } -function handleGraphQLResult( - result: PromiseOrValue | GraphQLResult>>, - newDeferredGroupedFieldSetRecords: ReadonlyArray, -): PromiseOrValue | GraphQLResult>> { - if (isPromise(result)) { - return result.then((resolvedResult) => - withNewDeferredGroupedFieldSets( - resolvedResult, - newDeferredGroupedFieldSetRecords, - ), - ); - } - return withNewDeferredGroupedFieldSets( - result, - newDeferredGroupedFieldSetRecords, - ); -} - -function withNewDeferredGroupedFieldSets( - results: ObjMap | GraphQLResult>, - newDeferredGroupedFieldSetRecords: ReadonlyArray, -): ObjMap | GraphQLResult> { - if (results instanceof GraphQLResult) { - return new GraphQLResult(results.result, [ - ...newDeferredGroupedFieldSetRecords, - ...results.futures, - ]); - } - return new GraphQLResult(results, newDeferredGroupedFieldSetRecords); -} - /** * Implements the "Executing selection sets" section of the spec * for fields that must be executed serially. @@ -528,8 +545,8 @@ function executeFieldsSerially( path: Path | undefined, groupedFieldSet: GroupedFieldSet, deferMap: ReadonlyMap, -): PromiseOrValue | GraphQLResult>> { - const fields = promiseReduce( +): PromiseOrValue> { + return promiseReduce( groupedFieldSet, (acc, [responseName, fieldGroup]) => { const fieldPath = addPath(path, responseName, parentType.name); @@ -546,51 +563,16 @@ function executeFieldsSerially( return acc; } if (isPromise(result)) { - return result.then((resolvedResult) => - withFieldFutures(acc, responseName, resolvedResult), - ); + return result.then((resolved) => { + acc[responseName] = resolved; + return acc; + }); } - return withFieldFutures(acc, responseName, result); - }, - { - result: Object.create(null), - futures: [] as Array, + acc[responseName] = result; + return acc; }, + Object.create(null), ); - - if (isPromise(fields)) { - return fields.then((resolved) => - withFutures(resolved.result, resolved.futures), - ); - } - return withFutures(fields.result, fields.futures); -} - -function withFutures( - result: T, - futures: ReadonlyArray, -): T | GraphQLResult { - return futures.length > 0 ? new GraphQLResult(result, futures) : result; -} - -function withFieldFutures( - acc: { - result: ObjMap; - futures: Array; - }, - responseName: string, - fieldResult: unknown, -): { - result: ObjMap; - futures: Array; -} { - if (fieldResult instanceof GraphQLResult) { - acc.result[responseName] = fieldResult.result; - acc.futures = [...acc.futures, ...fieldResult.futures]; - return acc; - } - acc.result[responseName] = fieldResult; - return acc; } /** @@ -605,9 +587,8 @@ function executeFields( groupedFieldSet: GroupedFieldSet, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): PromiseOrValue | GraphQLResult>> { +): PromiseOrValue> { const results = Object.create(null); - let futures: ReadonlyArray = []; let containsPromise = false; try { @@ -624,12 +605,7 @@ function executeFields( ); if (result !== undefined) { - if (result instanceof GraphQLResult) { - results[responseName] = result.result; - futures = [...futures, ...result.futures]; - } else { - results[responseName] = result; - } + results[responseName] = result; if (isPromise(result)) { containsPromise = true; } @@ -647,27 +623,13 @@ function executeFields( // If there are no promises, we can just return the object and any futures if (!containsPromise) { - return withFutures(results, futures); + return results; } // Otherwise, results is a map from field name to the result of resolving that // field, which is possibly a promise. Return a promise that will return this // same map, but with any promises replaced with the values they resolved to. - return promiseForObject( - results, - (acc, key, value) => { - if (value instanceof GraphQLResult) { - acc.result[key] = value.result; - acc.futures = [...acc.futures, ...value.futures]; - return; - } - acc.result[key] = value; - }, - { - result: Object.create(null), - futures, - }, - ).then((resolved) => withFutures(resolved.result, resolved.futures)); + return promiseForObject(results); } function toNodes(fieldGroup: FieldGroup): ReadonlyArray { @@ -822,7 +784,9 @@ function handleFieldError( // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. - (incrementalContext?.errors ?? exeContext.errors).push(error); + const { errors, errorPaths } = incrementalContext ?? exeContext; + errors.push(error); + errorPaths.add(path); } /** @@ -1067,11 +1031,10 @@ async function completeAsyncIteratorValue( asyncIterator: AsyncIterator, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): Promise | GraphQLResult>> { +): Promise> { const streamUsage = getStreamUsage(exeContext, fieldGroup, path); let containsPromise = false; const completedResults: Array = []; - const futures: Array = []; let index = 0; // eslint-disable-next-line no-constant-condition while (true) { @@ -1104,7 +1067,7 @@ async function completeAsyncIteratorValue( ), ); - futures.push(firstStreamItems); + (incrementalContext ?? exeContext).futures.push(firstStreamItems); break; } @@ -1124,7 +1087,6 @@ async function completeAsyncIteratorValue( completeListItemValue( iteration.value, completedResults, - futures, exeContext, itemType, fieldGroup, @@ -1139,28 +1101,7 @@ async function completeAsyncIteratorValue( index += 1; } - return containsPromise - ? withListFutures(completedResults, futures) - : withFutures(completedResults, futures); -} - -async function withListFutures( - promisedResults: Array>, - futures: Array, -) { - const resolvedResults = await Promise.all(promisedResults); - for (let i = 0; i < resolvedResults.length; i++) { - const completedItem = resolvedResults[i]; - /* c8 ignore next 4 */ - // TODO: add test case for a list item returning a promise that has futures - if (completedItem instanceof GraphQLResult) { - futures.push(...completedItem.futures); - resolvedResults[i] = completedItem.result; - } else { - resolvedResults[i] = completedItem; - } - } - return withFutures(resolvedResults, futures); + return containsPromise ? Promise.all(completedResults) : completedResults; } /** @@ -1176,9 +1117,7 @@ function completeListValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): PromiseOrValue< - ReadonlyArray | GraphQLResult> -> { +): PromiseOrValue> { const itemType = returnType.ofType; if (isAsyncIterable(result)) { @@ -1208,7 +1147,6 @@ function completeListValue( // where the list contains no Promises by avoiding creating another Promise. let containsPromise = false; const completedResults: Array = []; - const futures: Array = []; let index = 0; const iterator = result[Symbol.iterator](); let iteration = iterator.next(); @@ -1239,8 +1177,7 @@ function completeListValue( ), ); - futures.push(firstStreamItems); - + (incrementalContext ?? exeContext).futures.push(firstStreamItems); break; } @@ -1252,7 +1189,6 @@ function completeListValue( completeListItemValue( item, completedResults, - futures, exeContext, itemType, fieldGroup, @@ -1270,9 +1206,7 @@ function completeListValue( iteration = iterator.next(); } - return containsPromise - ? withListFutures(completedResults, futures) - : withFutures(completedResults, futures); + return containsPromise ? Promise.all(completedResults) : completedResults; } /** @@ -1283,7 +1217,6 @@ function completeListValue( function completeListItemValue( item: unknown, completedResults: Array, - futures: Array, exeContext: ExecutionContext, itemType: GraphQLOutputType, fieldGroup: FieldGroup, @@ -1341,12 +1274,7 @@ function completeListItemValue( return true; } - if (completedItem instanceof GraphQLResult) { - completedResults.push(completedItem.result); - futures.push(...completedItem.futures); - } else { - completedResults.push(completedItem); - } + completedResults.push(completedItem); } catch (rawError) { handleFieldError( rawError, @@ -1393,7 +1321,7 @@ function completeAbstractValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): PromiseOrValue | GraphQLResult>> { +): PromiseOrValue> { const resolveTypeFn = returnType.resolveType ?? exeContext.typeResolver; const contextValue = exeContext.contextValue; const runtimeType = resolveTypeFn(result, contextValue, info, returnType); @@ -1506,7 +1434,7 @@ function completeObjectValue( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): PromiseOrValue | GraphQLResult>> { +): PromiseOrValue> { // If there is an isTypeOf predicate function, call it with the // current result. If isTypeOf returns false, then raise an error rather // than continuing execution. @@ -1629,7 +1557,7 @@ function collectAndExecuteSubfields( result: unknown, incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap, -): PromiseOrValue | GraphQLResult>> { +): PromiseOrValue> { // Collect sub-fields to execute to complete this value. const { groupedFieldSet, newGroupedFieldSets, newDeferUsages } = buildSubFieldPlan( @@ -1661,7 +1589,9 @@ function collectAndExecuteSubfields( newDeferMap, ); - return handleGraphQLResult(subFields, newDeferredGroupedFieldSetRecords); + (incrementalContext ?? exeContext).futures.push( + ...newDeferredGroupedFieldSetRecords, + ); } return subFields; } @@ -2008,9 +1938,9 @@ function executeDeferredGroupedFieldSet( incrementalContext: IncrementalContext, deferMap: ReadonlyMap, ): PromiseOrValue { - let result; + let data; try { - result = executeFields( + data = executeFields( exeContext, parentType, sourceValue, @@ -2029,50 +1959,36 @@ function executeDeferredGroupedFieldSet( }; } - if (isPromise(result)) { - return result.then( - (resolved) => - buildDeferredGroupedFieldSetResult( - deferredFragmentRecords, - pathToArray(path), - resolved, - incrementalContext.errors, - ), + const { errors, errorPaths, futures } = incrementalContext; + + if (isPromise(data)) { + return data.then( + (resolved) => ({ + deferredFragmentRecords, + path: pathToArray(path), + data: resolved, + futures: filterFutures(errorPaths, futures), + errors, + }), (error) => { incrementalContext.errors.push(error); return { deferredFragmentRecords, path: pathToArray(path), data: null, - errors: incrementalContext.errors, + errors, }; }, ); } - return buildDeferredGroupedFieldSetResult( + return { deferredFragmentRecords, - pathToArray(path), - result, - incrementalContext.errors, - ); -} - -function buildDeferredGroupedFieldSetResult( - deferredFragmentRecords: ReadonlyArray, - path: Array, - result: ObjMap | GraphQLResult>, - errors: ReadonlyArray, -): DeferredGroupedFieldSetResult { - return result instanceof GraphQLResult - ? { - deferredFragmentRecords, - path, - data: result.result, - futures: result.futures, - errors, - } - : { deferredFragmentRecords, path, data: result, errors }; + path: pathToArray(path), + data, + futures: filterFutures(errorPaths, futures), + errors, + }; } function getDeferredFragmentRecords( @@ -2256,6 +2172,7 @@ function completeStreamItems( info: GraphQLResolveInfo, itemType: GraphQLOutputType, ): PromiseOrValue { + const { errors, errorPaths, futures } = incrementalContext; if (isPromise(item)) { return completePromisedValue( exeContext, @@ -2267,18 +2184,18 @@ function completeStreamItems( incrementalContext, new Map(), ).then( - (resolvedItem) => - buildStreamItemsResult( - streamRecord, - resolvedItem, - incrementalContext.errors, - ), + (resolvedItem) => ({ + streamRecord, + items: [resolvedItem], + futures: filterFutures(errorPaths, futures), + errors, + }), (error) => { - incrementalContext.errors.push(error); + errors.push(error); return { streamRecord, items: null, - errors: incrementalContext.errors, + errors, }; }, ); @@ -2313,7 +2230,7 @@ function completeStreamItems( return { streamRecord, items: null, - errors: incrementalContext.errors, + errors, }; } @@ -2331,41 +2248,27 @@ function completeStreamItems( return null; }) .then( - (resolvedItem) => - buildStreamItemsResult( - streamRecord, - resolvedItem, - incrementalContext.errors, - ), + (resolvedItem) => ({ + streamRecord, + items: [resolvedItem], + futures: filterFutures(errorPaths, futures), + errors, + }), (error) => { incrementalContext.errors.push(error); return { streamRecord, items: null, - errors: incrementalContext.errors, + errors, }; }, ); } - return buildStreamItemsResult( + return { streamRecord, - completedItem, - incrementalContext.errors, - ); -} - -function buildStreamItemsResult( - streamRecord: StreamRecord, - result: unknown, - errors: ReadonlyArray, -): StreamItemsResult { - return result instanceof GraphQLResult - ? { - streamRecord, - items: [result.result], - futures: result.futures, - errors, - } - : { streamRecord, items: [result], errors }; + items: [completedItem], + futures: filterFutures(errorPaths, futures), + errors, + }; } diff --git a/src/jsutils/promiseForObject.ts b/src/jsutils/promiseForObject.ts index ef0390f49f1..ff48d9f2180 100644 --- a/src/jsutils/promiseForObject.ts +++ b/src/jsutils/promiseForObject.ts @@ -2,19 +2,21 @@ import type { ObjMap } from './ObjMap.js'; /** * This function transforms a JS object `ObjMap>` into - * a `Promise` + * a `Promise>` + * + * This is akin to bluebird's `Promise.props`, but implemented only using + * `Promise.all` so it will work with any implementation of ES6 promises. */ -export async function promiseForObject( +export async function promiseForObject( object: ObjMap>, - callbackFn: (accumulator: U, key: string, currentValue: T) => void, - acc: U, -): Promise { +): Promise> { const keys = Object.keys(object); const values = Object.values(object); const resolvedValues = await Promise.all(values); + const resolvedObject = Object.create(null); for (let i = 0; i < keys.length; ++i) { - callbackFn(acc, keys[i], resolvedValues[i]); + resolvedObject[keys[i]] = resolvedValues[i]; } - return acc; + return resolvedObject; }