diff --git a/src/execution/__tests__/abort-signal-test.ts b/src/execution/__tests__/abort-signal-test.ts index ad9ba6c332..e229bdf9ef 100644 --- a/src/execution/__tests__/abort-signal-test.ts +++ b/src/execution/__tests__/abort-signal-test.ts @@ -349,7 +349,7 @@ describe('Execute: Cancellation', () => { }); }); - it('should stop deferred execution when aborted mid-execution', async () => { + it('should stop deferred execution when aborted prior to initiation of deferred execution', async () => { const abortController = new AbortController(); const document = parse(` query { @@ -358,9 +358,7 @@ describe('Execute: Cancellation', () => { ... on Todo @defer { text author { - ... on Author @defer { - id - } + id } } } @@ -428,6 +426,84 @@ describe('Execute: Cancellation', () => { ]); }); + it('should stop deferred execution when aborted within deferred execution', async () => { + const abortController = new AbortController(); + const document = parse(` + query { + ... on Query @defer { + todo { + id + text + author { + id + } + } + } + } + `); + + const resultPromise = complete( + document, + { + todo: async () => + Promise.resolve({ + id: '1', + text: 'hello world', + author: async () => + /* c8 ignore next 2 */ + Promise.resolve({ + id: () => expect.fail('Should not be called'), + }), + }), + }, + abortController.signal, + ); + + await resolveOnNextTick(); + await resolveOnNextTick(); + await resolveOnNextTick(); + + abortController.abort(); + + const result = await resultPromise; + + expectJSON(result).toDeepEqual([ + { + data: {}, + pending: [{ id: '0', path: [] }], + hasNext: true, + }, + { + incremental: [ + { + data: { + todo: { + id: '1', + text: 'hello world', + author: null, + }, + }, + errors: [ + { + locations: [ + { + column: 13, + line: 7, + }, + ], + message: 'This operation was aborted', + path: ['todo', 'author'], + }, + ], + id: '0', + }, + ], + completed: [{ id: '0' }], + hasNext: false, + }, + ]); + }); + it('should stop the execution when aborted mid-mutation', async () => { const abortController = new AbortController(); const document = parse(` diff --git a/src/execution/__tests__/nonnull-test.ts b/src/execution/__tests__/nonnull-test.ts index ff08aafd73..546bf24bfe 100644 --- a/src/execution/__tests__/nonnull-test.ts +++ b/src/execution/__tests__/nonnull-test.ts @@ -2,6 +2,7 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; +import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; import type { PromiseOrValue } from '../../jsutils/PromiseOrValue.js'; @@ -526,6 +527,93 @@ describe('Execute: handles non-nullable types', () => { }); }); + describe('cancellation with null bubbling', () => { + function nestedPromise(n: number): string { + return n > 0 ? `promiseNest { ${nestedPromise(n - 1)} }` : 'promise'; + } + it('returns both errors if insufficiently nested', async () => { + const query = ` + { + promiseNonNull, + ${nestedPromise(3)} + } + `; + + const result = await executeQuery(query, throwingData); + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: 'promise', + path: ['promiseNest', 'promiseNest', 'promiseNest', 'promise'], + locations: [{ line: 4, column: 51 }], + }, + { + message: 'promiseNonNull', + path: ['promiseNonNull'], + locations: [{ line: 3, column: 9 }], + }, + ], + }); + }); + + it('returns only a single error if sufficiently nested', async () => { + const query = ` + { + promiseNonNull, + ${nestedPromise(4)} + } + `; + + const result = await executeQuery(query, throwingData); + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + // does not include syncNullError because result returns prior to it being added + { + message: 'promiseNonNull', + path: ['promiseNonNull'], + locations: [{ line: 3, column: 11 }], + }, + ], + }); + }); + + it('keeps running despite error', async () => { + const query = ` + { + promiseNonNull, + ${nestedPromise(10)} + } + `; + + let counter = 0; + const rootValue = { + ...throwingData, + promiseNest() { + return new Promise((resolve) => { + counter++; + resolve(rootValue); + }); + }, + }; + const result = await executeQuery(query, rootValue); + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: 'promiseNonNull', + path: ['promiseNonNull'], + locations: [{ line: 3, column: 11 }], + }, + ], + }); + const counterAtExecutionEnd = counter; + await resolveOnNextTick(); + expect(counter).to.equal(counterAtExecutionEnd); + }); + }); + describe('Handles non-null argument', () => { const schemaWithNonNullArg = new GraphQLSchema({ query: new GraphQLObjectType({ diff --git a/src/execution/execute.ts b/src/execution/execute.ts index beef2fbb80..1ac016bb15 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -162,11 +162,13 @@ export interface ValidatedExecutionArgs { export interface ExecutionContext { validatedExecutionArgs: ValidatedExecutionArgs; + internalAbortSignal: AbortSignal; errors: Array | undefined; cancellableStreams: Set | undefined; } interface IncrementalContext { + internalAbortSignal: AbortSignal; errors: Array | undefined; deferUsageSet?: DeferUsageSet | undefined; } @@ -307,21 +309,31 @@ export function executeQueryOrMutationOrSubscriptionEvent( export function experimentalExecuteQueryOrMutationOrSubscriptionEvent( validatedExecutionArgs: ValidatedExecutionArgs, ): PromiseOrValue { + const { + schema, + fragments, + rootValue, + operation, + variableValues, + hideSuggestions, + abortSignal, + } = validatedExecutionArgs; + + const internalAbortController = new AbortController(); + const addAbortListener = () => { + internalAbortController.abort(abortSignal?.reason); + abortSignal?.removeEventListener('abort', addAbortListener); + }; + abortSignal?.addEventListener('abort', addAbortListener); + const exeContext: ExecutionContext = { validatedExecutionArgs, + internalAbortSignal: internalAbortController.signal, errors: undefined, cancellableStreams: undefined, }; - try { - const { - schema, - fragments, - rootValue, - operation, - variableValues, - hideSuggestions, - } = validatedExecutionArgs; + try { const rootType = schema.getRootType(operation.operation); if (rootType == null) { throw new GraphQLError( @@ -360,19 +372,27 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent( if (isPromise(graphqlWrappedResult)) { return graphqlWrappedResult.then( - (resolved) => buildDataResponse(exeContext, resolved[0], resolved[1]), - (error: unknown) => ({ - data: null, - errors: withError(exeContext.errors, error as GraphQLError), - }), + (resolved) => { + internalAbortController.abort(); + return buildDataResponse(exeContext, resolved[0], resolved[1]); + }, + (error: unknown) => { + internalAbortController.abort(); + return { + data: null, + errors: withError(exeContext.errors, error as GraphQLError), + }; + }, ); } + internalAbortController.abort(); return buildDataResponse( exeContext, graphqlWrappedResult[0], graphqlWrappedResult[1], ); } catch (error) { + internalAbortController.abort(); return { data: null, errors: withError(exeContext.errors, error) }; } } @@ -665,10 +685,10 @@ function executeFieldsSerially( groupedFieldSet, (graphqlWrappedResult, [responseName, fieldDetailsList]) => { const fieldPath = addPath(path, responseName, parentType.name); - const abortSignal = exeContext.validatedExecutionArgs.abortSignal; - if (abortSignal?.aborted) { + const internalAbortSignal = exeContext.internalAbortSignal; + if (internalAbortSignal?.aborted) { handleFieldError( - abortSignal.reason, + internalAbortSignal.reason, exeContext, parentType, fieldDetailsList, @@ -798,7 +818,7 @@ function executeField( deferMap: ReadonlyMap | undefined, ): PromiseOrValue> | undefined { const validatedExecutionArgs = exeContext.validatedExecutionArgs; - const { schema, contextValue, variableValues, hideSuggestions, abortSignal } = + const { schema, contextValue, variableValues, hideSuggestions } = validatedExecutionArgs; const fieldName = fieldDetailsList[0].node.name.value; const fieldDef = schema.getField(parentType, fieldName); @@ -833,7 +853,13 @@ function executeField( // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. - const result = resolveFn(source, args, contextValue, info, abortSignal); + const result = resolveFn( + source, + args, + contextValue, + info, + (incrementalContext ?? exeContext).internalAbortSignal, + ); if (isPromise(result)) { return completePromisedValue( @@ -1728,11 +1754,11 @@ function completeObjectValue( incrementalContext: IncrementalContext | undefined, deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { - const validatedExecutionArgs = exeContext.validatedExecutionArgs; - const abortSignal = validatedExecutionArgs.abortSignal; - if (abortSignal?.aborted) { + const internalAbortSignal = (incrementalContext ?? exeContext) + .internalAbortSignal; + if (internalAbortSignal?.aborted) { throw locatedError( - abortSignal.reason, + internalAbortSignal.reason, toNodes(fieldDetailsList), pathToArray(path), ); @@ -1744,7 +1770,7 @@ function completeObjectValue( if (returnType.isTypeOf) { const isTypeOf = returnType.isTypeOf( result, - validatedExecutionArgs.contextValue, + exeContext.validatedExecutionArgs.contextValue, info, ); @@ -1955,12 +1981,17 @@ export const defaultTypeResolver: GraphQLTypeResolver = * of calling that function while passing along args and context value. */ export const defaultFieldResolver: GraphQLFieldResolver = - function (source: any, args, contextValue, info, abortSignal) { + function (source: any, args, contextValue, info, internalAbortSignal) { // ensure source is a value for which property access is acceptable. if (isObjectLike(source) || typeof source === 'function') { const property = source[info.fieldName]; if (typeof property === 'function') { - return source[info.fieldName](args, contextValue, info, abortSignal); + return source[info.fieldName]( + args, + contextValue, + info, + internalAbortSignal, + ); } return property; } @@ -2245,6 +2276,9 @@ function collectExecutionGroups( path, groupedFieldSet, { + internalAbortSignal: prepareInternalAbortSignal( + exeContext.validatedExecutionArgs.abortSignal, + ), errors: undefined, deferUsageSet, }, @@ -2267,6 +2301,26 @@ function collectExecutionGroups( return newPendingExecutionGroups; } +function prepareInternalAbortSignal( + externalAbortSignal: AbortSignal | undefined, +): AbortSignal { + if (!externalAbortSignal) { + return new AbortController().signal; + } + + if (externalAbortSignal.aborted) { + return AbortSignal.abort(externalAbortSignal.reason); + } + + const internalAbortController = new AbortController(); + const addAbortListener = () => { + internalAbortController.abort(externalAbortSignal?.reason); + externalAbortSignal.removeEventListener('abort', addAbortListener); + }; + externalAbortSignal.addEventListener('abort', addAbortListener); + return internalAbortController.signal; +} + function shouldDefer( parentDeferUsages: undefined | DeferUsageSet, deferUsages: DeferUsageSet, @@ -2378,12 +2432,18 @@ function buildSyncStreamItemQueue( const firstExecutor = () => { const initialPath = addPath(streamPath, initialIndex, undefined); + const firstStreamItem = new BoxedPromiseOrValue( completeStreamItem( initialPath, initialItem, exeContext, - { errors: undefined }, + { + internalAbortSignal: prepareInternalAbortSignal( + exeContext.validatedExecutionArgs.abortSignal, + ), + errors: undefined, + }, fieldDetailsList, info, itemType, @@ -2414,7 +2474,12 @@ function buildSyncStreamItemQueue( itemPath, value, exeContext, - { errors: undefined }, + { + internalAbortSignal: prepareInternalAbortSignal( + exeContext.validatedExecutionArgs.abortSignal, + ), + errors: undefined, + }, fieldDetailsList, info, itemType, @@ -2506,7 +2571,12 @@ async function getNextAsyncStreamItemResult( itemPath, iteration.value, exeContext, - { errors: undefined }, + { + internalAbortSignal: prepareInternalAbortSignal( + exeContext.validatedExecutionArgs.abortSignal, + ), + errors: undefined, + }, fieldDetailsList, info, itemType,