From e449cae02b64209005bf5903c8c312d2ea4c4096 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 20:56:41 +0000 Subject: [PATCH] Always create abortController for each execution, listen to external (passed in by client) abort signal and abort our own signal after the execution is ended. Polyfill AbortController --- src/execution/__tests__/executor-test.ts | 170 +++++++++-------------- src/execution/execute.ts | 67 +++++---- src/jsutils/AbortController.ts | 31 +++++ 3 files changed, 135 insertions(+), 133 deletions(-) create mode 100644 src/jsutils/AbortController.ts diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index ca2f37225b..ec953d0db1 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -4,6 +4,7 @@ import { describe, it } from 'mocha'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; +import { AbortController } from '../../jsutils/AbortController.js'; import { inspect } from '../../jsutils/inspect.js'; import { Kind } from '../../language/kinds.js'; @@ -1314,59 +1315,59 @@ describe('Execute: Handles basic execution tasks', () => { expect(possibleTypes).to.deep.equal([fooObject]); }); - describe('Abort execution', () => { - it('stops execution and throws an error when signal is aborted', async () => { - /** - * This test has 3 resolvers nested in each other. - * Every resolve function waits 200ms before returning data. - * - * The test waits for the first resolver and half of the 2nd resolver execution time (200ms + 100ms) - * and then aborts the execution. - * - * 2nd resolver execution finishes, and we then expect to not execute the 3rd resolver - * and to get an error about aborted operation. - */ - - const WAIT_MS_BEFORE_RESOLVING = 200; - const ABORT_IN_MS_AFTER_STARTING_EXECUTION = - WAIT_MS_BEFORE_RESOLVING + WAIT_MS_BEFORE_RESOLVING / 2; - - const schema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { - resolvesIn500ms: { - type: new GraphQLObjectType({ - name: 'ResolvesIn500ms', - fields: { - resolvesIn400ms: { - type: new GraphQLObjectType({ - name: 'ResolvesIn400ms', - fields: { - shouldNotBeResolved: { - type: GraphQLString, - resolve: () => { - throw new Error('This should not be executed!'); - }, + it('stops execution and throws an error when signal is aborted', async () => { + /** + * This test has 3 resolvers nested in each other. + * Every resolve function waits 200ms before returning data. + * + * The test waits for the first resolver and half of the 2nd resolver execution time (200ms + 100ms) + * and then aborts the execution. + * + * 2nd resolver execution finishes, and we then expect to not execute the 3rd resolver + * and to get an error about aborted operation. + */ + + const WAIT_MS_BEFORE_RESOLVING = 200; + const ABORT_IN_MS_AFTER_STARTING_EXECUTION = + WAIT_MS_BEFORE_RESOLVING + WAIT_MS_BEFORE_RESOLVING / 2; + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + resolvesIn500ms: { + type: new GraphQLObjectType({ + name: 'ResolvesIn500ms', + fields: { + resolvesIn400ms: { + type: new GraphQLObjectType({ + name: 'ResolvesIn400ms', + fields: { + shouldNotBeResolved: { + type: GraphQLString, + /* c8 ignore next 3 */ + resolve: () => { + throw new Error('This should not be executed!'); }, }, + }, + }), + resolve: () => + new Promise((resolve) => { + setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); }), - resolve: () => - new Promise((resolve) => { - setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); - }), - }, }, + }, + }), + resolve: () => + new Promise((resolve) => { + setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); }), - resolve: () => - new Promise((resolve) => { - setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); - }), - }, }, - }), - }); - const document = parse(` + }, + }), + }); + const document = parse(` query { resolvesIn500ms { resolvesIn400ms { @@ -1376,67 +1377,22 @@ describe('Execute: Handles basic execution tasks', () => { } `); - const abortController = new AbortController(); - const executionPromise = execute({ - schema, - document, - signal: abortController.signal, - }); - - setTimeout( - () => abortController.abort(), - ABORT_IN_MS_AFTER_STARTING_EXECUTION, - ); - - const result = await executionPromise; - expect(result.errors?.[0].message).to.eq( - 'Execution aborted. Reason: AbortError: This operation was aborted', - ); - expect(result.data).to.eql({ - resolvesIn500ms: { resolvesIn400ms: null }, - }); - }); - - const abortMessageTestInputs = [ - { message: 'Aborted from somewhere', reason: 'Aborted from somewhere' }, - { message: undefined, reason: 'AbortError: This operation was aborted' }, - ]; - - for (const { message, reason } of abortMessageTestInputs) { - it('aborts with "Reason:" in the error message', async () => { - const schema = new GraphQLSchema({ - query: new GraphQLObjectType({ - name: 'Query', - fields: { - a: { - type: GraphQLString, - resolve: () => - new Promise((resolve) => { - setTimeout(() => resolve({}), 100); - }), - }, - }, - }), - }); - - const document = parse(` - query { a } - `); - - const abortController = new AbortController(); - const executionPromise = execute({ - schema, - document, - signal: abortController.signal, - }); + const abortController = new AbortController(); + const executionPromise = execute({ + schema, + document, + signal: abortController.signal, + }); - abortController.abort(message); + setTimeout( + () => abortController.abort(), + ABORT_IN_MS_AFTER_STARTING_EXECUTION, + ); - const { errors } = await executionPromise; - expect(errors?.[0].message).to.eq( - `Execution aborted. Reason: ${reason}`, - ); - }); - } + const result = await executionPromise; + expect(result.errors?.[0].message).to.eq('Execution aborted.'); + expect(result.data).to.eql({ + resolvesIn500ms: { resolvesIn400ms: null }, + }); }); }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index d969a5c09b..5c4404d703 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,3 +1,8 @@ +import type { + IAbortController, + IAbortSignal, +} from '../jsutils/AbortController.js'; +import { AbortController } from '../jsutils/AbortController.js'; import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; import { isAsyncIterable } from '../jsutils/isAsyncIterable.js'; @@ -122,9 +127,10 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; errors: Array; subsequentPayloads: Set; - signal: Maybe<{ - isAborted: boolean; - instance: AbortSignal; + abortion: Maybe<{ + passedInAbortSignal: IAbortSignal; + executionAbortController: IAbortController; + executionAbortSignal: IAbortSignal; }>; } @@ -265,7 +271,7 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; - signal?: AbortSignal; + signal?: IAbortSignal; } const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = @@ -342,28 +348,25 @@ export function experimentalExecuteIncrementally( return executeImpl(exeContext); } -function subscribeToAbortSignal(exeContext: ExecutionContext): { - unsubscribeFromAbortSignal: () => void; -} { - const onAbort = () => { - if ('signal' in exeContext && exeContext.signal) { - exeContext.signal.isAborted = true; - } - }; +function subscribeToAbortSignal(exeContext: ExecutionContext): () => void { + const { abortion } = exeContext; + if (!abortion) { + return () => null; + } - exeContext.signal?.instance.addEventListener('abort', onAbort); + const onAbort = () => abortion.executionAbortController.abort(abortion); + abortion.passedInAbortSignal.addEventListener('abort', onAbort); - return { - unsubscribeFromAbortSignal: () => { - exeContext.signal?.instance.removeEventListener('abort', onAbort); - }, + return () => { + abortion.passedInAbortSignal.removeEventListener('abort', onAbort); + abortion.executionAbortController.abort(); }; } function executeImpl( exeContext: ExecutionContext, ): PromiseOrValue { - const { unsubscribeFromAbortSignal } = subscribeToAbortSignal(exeContext); + const unsubscribeFromAbortSignal = subscribeToAbortSignal(exeContext); // Return a Promise that will eventually resolve to the data described by // The "Response" section of the GraphQL specification. @@ -473,7 +476,7 @@ export function buildExecutionContext( fieldResolver, typeResolver, subscribeFieldResolver, - signal, + signal: passedInAbortSignal, } = args; // If the schema used for execution is invalid, throw an error. @@ -539,7 +542,23 @@ export function buildExecutionContext( subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, subsequentPayloads: new Set(), errors: [], - signal: signal ? { instance: signal, isAborted: false } : null, + abortion: getContextAbortionEntities(passedInAbortSignal), + }; +} + +function getContextAbortionEntities( + passedInAbortSignal: Maybe, +): ExecutionContext['abortion'] { + if (!passedInAbortSignal) { + return null; + } + + const executionAbortController = new AbortController(); + + return { + passedInAbortSignal, + executionAbortController, + executionAbortSignal: executionAbortController.signal, }; } @@ -873,12 +892,8 @@ function completeValue( result: unknown, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { - if (exeContext.signal?.isAborted) { - throw new GraphQLError( - `Execution aborted. Reason: ${ - exeContext.signal.instance.reason ?? 'Unknown.' - }`, - ); + if (exeContext.abortion?.executionAbortSignal.aborted) { + throw new GraphQLError('Execution aborted.'); } // If result is an Error, throw a located error. diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts new file mode 100644 index 0000000000..40ed4e48e7 --- /dev/null +++ b/src/jsutils/AbortController.ts @@ -0,0 +1,31 @@ +export interface IAbortSignal { + aborted: boolean; + addEventListener: (type: string, handler: () => void) => void; + removeEventListener: (type: string, handler: () => void) => void; +} + +export interface IAbortController { + signal: IAbortSignal; + abort: (reason?: any) => void; +} + +/* c8 ignore start */ +export const AbortController: new () => IAbortController = + // eslint-disable-next-line no-undef + global.AbortController || + class MockAbortController implements IAbortController { + private _signal: IAbortSignal = { + aborted: false, + addEventListener: () => null, + removeEventListener: () => null, + }; + + public get signal(): IAbortSignal { + return this._signal; + } + + public abort(): void { + this._signal.aborted = true; + } + }; +/* c8 ignore stop */