From f8b148f840fec29067ce34413948846c24232b85 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Mon, 28 Nov 2022 22:01:27 +0000 Subject: [PATCH 01/17] accept abortSignal in execute method and check if signal is aborted when resolving every field --- src/execution/__tests__/executor-test.ts | 126 +++++++++++++++++++++++ src/execution/execute.ts | 43 ++++++++ 2 files changed, 169 insertions(+) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 5e25dddb5f..ca2f37225b 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -1313,4 +1313,130 @@ describe('Execute: Handles basic execution tasks', () => { expect(result).to.deep.equal({ data: { foo: { bar: 'bar' } } }); 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!'); + }, + }, + }, + }), + resolve: () => + new Promise((resolve) => { + setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); + }), + }, + }, + }), + resolve: () => + new Promise((resolve) => { + setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); + }), + }, + }, + }), + }); + const document = parse(` + query { + resolvesIn500ms { + resolvesIn400ms { + shouldNotBeResolved + } + } + } + `); + + 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, + }); + + abortController.abort(message); + + const { errors } = await executionPromise; + expect(errors?.[0].message).to.eq( + `Execution aborted. Reason: ${reason}`, + ); + }); + } + }); }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 1bc6c4267b..d969a5c09b 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -122,6 +122,10 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; errors: Array; subsequentPayloads: Set; + signal: Maybe<{ + isAborted: boolean; + instance: AbortSignal; + }>; } /** @@ -261,6 +265,7 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; + signal?: AbortSignal; } const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = @@ -337,9 +342,29 @@ export function experimentalExecuteIncrementally( return executeImpl(exeContext); } +function subscribeToAbortSignal(exeContext: ExecutionContext): { + unsubscribeFromAbortSignal: () => void; +} { + const onAbort = () => { + if ('signal' in exeContext && exeContext.signal) { + exeContext.signal.isAborted = true; + } + }; + + exeContext.signal?.instance.addEventListener('abort', onAbort); + + return { + unsubscribeFromAbortSignal: () => { + exeContext.signal?.instance.removeEventListener('abort', onAbort); + }, + }; +} + function executeImpl( exeContext: ExecutionContext, ): PromiseOrValue { + const { unsubscribeFromAbortSignal } = subscribeToAbortSignal(exeContext); + // Return a Promise that will eventually resolve to the data described by // The "Response" section of the GraphQL specification. // @@ -356,6 +381,8 @@ function executeImpl( if (isPromise(result)) { return result.then( (data) => { + unsubscribeFromAbortSignal(); + const initialResult = buildResponse(data, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -369,11 +396,16 @@ function executeImpl( return initialResult; }, (error) => { + unsubscribeFromAbortSignal(); + exeContext.errors.push(error); return buildResponse(null, exeContext.errors); }, ); } + + unsubscribeFromAbortSignal(); + const initialResult = buildResponse(result, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -386,6 +418,7 @@ function executeImpl( } return initialResult; } catch (error) { + unsubscribeFromAbortSignal(); exeContext.errors.push(error); return buildResponse(null, exeContext.errors); } @@ -440,6 +473,7 @@ export function buildExecutionContext( fieldResolver, typeResolver, subscribeFieldResolver, + signal, } = args; // If the schema used for execution is invalid, throw an error. @@ -505,6 +539,7 @@ export function buildExecutionContext( subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, subsequentPayloads: new Set(), errors: [], + signal: signal ? { instance: signal, isAborted: false } : null, }; } @@ -838,6 +873,14 @@ function completeValue( result: unknown, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { + if (exeContext.signal?.isAborted) { + throw new GraphQLError( + `Execution aborted. Reason: ${ + exeContext.signal.instance.reason ?? 'Unknown.' + }`, + ); + } + // If result is an Error, throw a located error. if (result instanceof Error) { throw result; From e449cae02b64209005bf5903c8c312d2ea4c4096 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 20:56:41 +0000 Subject: [PATCH 02/17] 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 */ From f47dee791741d6d0e9d4f4b2b8bfb9d5429ffca7 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 20:59:58 +0000 Subject: [PATCH 03/17] Throw an error if client provides abort signal for subscriptions --- src/execution/execute.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 5c4404d703..854586176b 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1631,6 +1631,14 @@ export function subscribe( ): PromiseOrValue< AsyncGenerator | ExecutionResult > { + // Until we have execution cancelling support in Subscriptions, + // throw an error if client provides abort signal. + if (args.signal) { + return { + errors: [new GraphQLError('Subscriptions do not support abort signals.')], + }; + } + // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. const exeContext = buildExecutionContext(args); From e4b51d377d08a01b34e82d28fa14ea3487c06527 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 21:09:18 +0000 Subject: [PATCH 04/17] Ignore aborted condition test coverage --- src/execution/execute.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 854586176b..9c8340cff9 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -892,6 +892,9 @@ function completeValue( result: unknown, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { + // Ignoring test coverage for abortion check since Node 14 doesn't support AbortSignal + // and this condition is never true. + /* c8 ignore next 3 */ if (exeContext.abortion?.executionAbortSignal.aborted) { throw new GraphQLError('Execution aborted.'); } From 8f84774296425752ccfbdd68f34c75356af3104c Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 21:18:42 +0000 Subject: [PATCH 05/17] ignore coverage and skip abortion test if node doesn't support AbortController --- src/execution/__tests__/executor-test.ts | 138 ++++++++++++----------- src/execution/execute.ts | 11 +- src/jsutils/AbortController.ts | 5 + 3 files changed, 85 insertions(+), 69 deletions(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index ec953d0db1..0ea61bea0b 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -4,7 +4,10 @@ import { describe, it } from 'mocha'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; -import { AbortController } from '../../jsutils/AbortController.js'; +import { + AbortController, + hasAbortControllerSupport, +} from '../../jsutils/AbortController.js'; import { inspect } from '../../jsutils/inspect.js'; import { Kind } from '../../language/kinds.js'; @@ -1315,59 +1318,60 @@ describe('Execute: Handles basic execution tasks', () => { expect(possibleTypes).to.deep.equal([fooObject]); }); - 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!'); + /* c8 ignore start */ + if (hasAbortControllerSupport) { + 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!'); + }, }, }, - }, - }), - 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 { @@ -1377,22 +1381,24 @@ 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.'); - expect(result.data).to.eql({ - resolvesIn500ms: { resolvesIn400ms: null }, - }); - }); + 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.'); + expect(result.data).to.eql({ + resolvesIn500ms: { resolvesIn400ms: null }, + }); + }); + } + /* c8 ignore stop */ }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 9c8340cff9..2e67be875f 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -348,6 +348,7 @@ export function experimentalExecuteIncrementally( return executeImpl(exeContext); } +/* c8 ignore start */ function subscribeToAbortSignal(exeContext: ExecutionContext): () => void { const { abortion } = exeContext; if (!abortion) { @@ -362,6 +363,7 @@ function subscribeToAbortSignal(exeContext: ExecutionContext): () => void { abortion.executionAbortController.abort(); }; } +/* c8 ignore stop */ function executeImpl( exeContext: ExecutionContext, @@ -546,6 +548,7 @@ export function buildExecutionContext( }; } +/* c8 ignore start */ function getContextAbortionEntities( passedInAbortSignal: Maybe, ): ExecutionContext['abortion'] { @@ -561,6 +564,7 @@ function getContextAbortionEntities( executionAbortSignal: executionAbortController.signal, }; } +/* c8 ignore stop */ function buildPerEventExecutionContext( exeContext: ExecutionContext, @@ -892,12 +896,11 @@ function completeValue( result: unknown, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { - // Ignoring test coverage for abortion check since Node 14 doesn't support AbortSignal - // and this condition is never true. - /* c8 ignore next 3 */ + /* c8 ignore start */ if (exeContext.abortion?.executionAbortSignal.aborted) { throw new GraphQLError('Execution aborted.'); } + /* c8 ignore stop */ // If result is an Error, throw a located error. if (result instanceof Error) { @@ -1636,11 +1639,13 @@ export function subscribe( > { // Until we have execution cancelling support in Subscriptions, // throw an error if client provides abort signal. + /* c8 ignore start */ if (args.signal) { return { errors: [new GraphQLError('Subscriptions do not support abort signals.')], }; } + /* c8 ignore stop */ // If a valid execution context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts index 40ed4e48e7..96293a3023 100644 --- a/src/jsutils/AbortController.ts +++ b/src/jsutils/AbortController.ts @@ -28,4 +28,9 @@ export const AbortController: new () => IAbortController = this._signal.aborted = true; } }; + +export const hasAbortControllerSupport = + // eslint-disable-next-line no-undef + Boolean(global.AbortController); + /* c8 ignore stop */ From b4751f45ccc0675b84fda43d466afa531743f651 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Wed, 7 Dec 2022 21:31:53 +0000 Subject: [PATCH 06/17] rename abort controller mock to DummyAbortController --- src/jsutils/AbortController.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts index 96293a3023..3657cf6353 100644 --- a/src/jsutils/AbortController.ts +++ b/src/jsutils/AbortController.ts @@ -9,11 +9,13 @@ export interface IAbortController { abort: (reason?: any) => void; } +// We need to polyfill abort controller in case of Node@14 which doesn't have it /* c8 ignore start */ export const AbortController: new () => IAbortController = // eslint-disable-next-line no-undef global.AbortController || - class MockAbortController implements IAbortController { + // This is a dummy implementation that doesn't actually abort anything + class DummyAbortController implements IAbortController { private _signal: IAbortSignal = { aborted: false, addEventListener: () => null, From b6b38aefe55b7ef775a097d0bd5d7d5025a284f1 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Thu, 8 Dec 2022 17:57:46 +0000 Subject: [PATCH 07/17] accept signal in `graphql` function and pass it down to `execute` --- src/execution/execute.ts | 2 +- src/graphql.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 2e67be875f..5e1f2ec65e 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -271,7 +271,7 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; - signal?: IAbortSignal; + signal?: Maybe; } const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = diff --git a/src/graphql.ts b/src/graphql.ts index 109bc75d92..2f9392d54e 100644 --- a/src/graphql.ts +++ b/src/graphql.ts @@ -1,3 +1,4 @@ +import type { IAbortSignal } from './jsutils/AbortController'; import { isPromise } from './jsutils/isPromise.js'; import type { Maybe } from './jsutils/Maybe.js'; import type { PromiseOrValue } from './jsutils/PromiseOrValue.js'; @@ -57,6 +58,9 @@ import { execute } from './execution/execute.js'; * A type resolver function to use when none is provided by the schema. * If not provided, the default type resolver is used (which looks for a * `__typename` field or alternatively calls the `isTypeOf` method). + * signal: + * An AbortSignal that can be used to abort the execution of the query. + * If the signal is aborted, the execution will stop and an abort error will be thrown. */ export interface GraphQLArgs { schema: GraphQLSchema; @@ -67,6 +71,7 @@ export interface GraphQLArgs { operationName?: Maybe; fieldResolver?: Maybe>; typeResolver?: Maybe>; + signal?: IAbortSignal; } export function graphql(args: GraphQLArgs): Promise { @@ -101,6 +106,7 @@ function graphqlImpl(args: GraphQLArgs): PromiseOrValue { operationName, fieldResolver, typeResolver, + signal, } = args; // Validate Schema @@ -133,5 +139,6 @@ function graphqlImpl(args: GraphQLArgs): PromiseOrValue { operationName, fieldResolver, typeResolver, + signal, }); } From 2baa1e3ae5230c6f7e32bc9cc2c267b80d39006e Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Wed, 25 Jan 2023 23:29:21 +0200 Subject: [PATCH 08/17] Remove setTimeout usages in tests --- src/__testUtils__/resolveOnNextTick.ts | 6 +- src/execution/__tests__/executor-test.ts | 147 +++++++++++++---------- 2 files changed, 87 insertions(+), 66 deletions(-) diff --git a/src/__testUtils__/resolveOnNextTick.ts b/src/__testUtils__/resolveOnNextTick.ts index 6dd50b3982..48e96a6721 100644 --- a/src/__testUtils__/resolveOnNextTick.ts +++ b/src/__testUtils__/resolveOnNextTick.ts @@ -1,3 +1,5 @@ -export function resolveOnNextTick(): Promise { - return Promise.resolve(undefined); +export function resolveOnNextTick(): Promise; +export function resolveOnNextTick(value: T): Promise; +export function resolveOnNextTick(value: unknown = undefined): Promise { + return Promise.resolve(value); } diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 0ea61bea0b..88cef968de 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -1318,87 +1318,106 @@ describe('Execute: Handles basic execution tasks', () => { expect(possibleTypes).to.deep.equal([fooObject]); }); - /* c8 ignore start */ if (hasAbortControllerSupport) { 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!'); - }, - }, - }, - }), - resolve: () => - new Promise((resolve) => { - setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); - }), - }, - }, - }), - resolve: () => - new Promise((resolve) => { - setTimeout(() => resolve({}), WAIT_MS_BEFORE_RESOLVING); - }), + const TestType: GraphQLObjectType = new GraphQLObjectType({ + name: 'TestType', + fields: () => ({ + resolveOnNextTick: { + type: TestType, + resolve: () => resolveOnNextTick({}), + }, + string: { + type: GraphQLString, + args: { + value: { type: new GraphQLNonNull(GraphQLString) }, }, + resolve: (_, { value }) => value, + }, + abortExecution: { + type: GraphQLString, + resolve: () => { + abortController.abort(); + return 'aborted'; + }, + }, + shouldNotBeResolved: { + type: GraphQLString, + /* c8 ignore next */ + resolve: () => 'This should not be executed!', }, }), }); + + const schema = new GraphQLSchema({ + query: TestType, + }); + const document = parse(` - query { - resolvesIn500ms { - resolvesIn400ms { - shouldNotBeResolved + query { + value1: string(value: "1") + resolveOnNextTick { + value2: string(value: "2") + resolveOnNextTick { + resolveOnNextTick { + shouldNotBeResolved + } + abortExecution + } + } + alternativeBranch: resolveOnNextTick { + value3: string(value: "3") + resolveOnNextTick { + shouldNotBeResolved + } } } - } - `); + `); const abortController = new AbortController(); - const executionPromise = execute({ + const result = await 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.'); - expect(result.data).to.eql({ - resolvesIn500ms: { resolvesIn400ms: null }, + expectJSON(result).toDeepEqual({ + data: { + value1: '1', + resolveOnNextTick: { + value2: '2', + resolveOnNextTick: { + abortExecution: null, + resolveOnNextTick: null, + }, + }, + alternativeBranch: { + resolveOnNextTick: null, + value3: '3', + }, + }, + errors: [ + { + message: 'Execution aborted.', + path: ['resolveOnNextTick', 'resolveOnNextTick', 'abortExecution'], + locations: [{ line: 10, column: 15 }], + }, + { + message: 'Execution aborted.', + path: ['alternativeBranch', 'resolveOnNextTick'], + locations: [{ line: 15, column: 13 }], + }, + { + message: 'Execution aborted.', + path: [ + 'resolveOnNextTick', + 'resolveOnNextTick', + 'resolveOnNextTick', + ], + locations: [{ line: 7, column: 15 }], + }, + ], }); }); } - /* c8 ignore stop */ }); From 256f780ba7b8dfd4b789891d238bb979a4a60e51 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Mon, 6 Feb 2023 23:45:16 +0200 Subject: [PATCH 09/17] Fix steam/defer + subscription and pass signal into resolvers --- src/execution/__tests__/executor-test.ts | 261 ++++++++++++------ .../__tests__/mapAsyncIterable-test.ts | 71 ++++- src/execution/execute.ts | 105 ++++--- src/execution/mapAsyncIterable.ts | 14 +- src/graphql.ts | 3 +- src/jsutils/AbortController.ts | 38 --- src/type/definition.ts | 6 + 7 files changed, 295 insertions(+), 203 deletions(-) delete mode 100644 src/jsutils/AbortController.ts diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 88cef968de..8a45a78ef0 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -4,10 +4,6 @@ import { describe, it } from 'mocha'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js'; -import { - AbortController, - hasAbortControllerSupport, -} from '../../jsutils/AbortController.js'; import { inspect } from '../../jsutils/inspect.js'; import { Kind } from '../../language/kinds.js'; @@ -226,6 +222,7 @@ describe('Execute: Handles basic execution tasks', () => { 'rootValue', 'operation', 'variableValues', + 'signal', ); const operation = document.definitions[0]; @@ -1318,106 +1315,192 @@ describe('Execute: Handles basic execution tasks', () => { expect(possibleTypes).to.deep.equal([fooObject]); }); - if (hasAbortControllerSupport) { - it('stops execution and throws an error when signal is aborted', async () => { - const TestType: GraphQLObjectType = new GraphQLObjectType({ - name: 'TestType', - fields: () => ({ - resolveOnNextTick: { - type: TestType, - resolve: () => resolveOnNextTick({}), - }, - string: { - type: GraphQLString, - args: { - value: { type: new GraphQLNonNull(GraphQLString) }, - }, - resolve: (_, { value }) => value, - }, - abortExecution: { - type: GraphQLString, - resolve: () => { - abortController.abort(); - return 'aborted'; - }, + it('stops execution and throws an error when signal is aborted', async () => { + // TODO: use real Event once we can finally drop node14 support + class MockAbortEvent implements Event { + cancelable = false; + bubbles = false; + composed = false; + currentTarget = null; + cancelBubble = false; + defaultPrevented = false; + isTrusted = true; + returnValue = false; + srcElement = null; + type = 'abort'; + eventPhase = 0; + timeStamp = 0; + AT_TARGET = 0; + BUBBLING_PHASE = 0; + CAPTURING_PHASE = 0; + NONE = 0; + + target: AbortSignal; + + constructor(abortSignal: AbortSignal) { + this.target = abortSignal; + } + + composedPath = () => { + throw new Error('Not mocked!'); + }; + + initEvent = () => { + throw new Error('Not mocked!'); + }; + + preventDefault = () => { + throw new Error(''); + }; + + stopImmediatePropagation = () => { + throw new Error(''); + }; + + stopPropagation = () => { + throw new Error(''); + }; + } + + class MockAbortSignal implements AbortSignal { + aborted: boolean = false; + onabort: ((ev: Event) => any) | null = null; + reason: unknown; + + throwIfAborted() { + if (this.aborted) { + throw this.reason; + } + } + + addEventListener(type: string, cb: unknown) { + expect(type).to.equal('abort'); + expect(this.onabort).to.equal(null); + expect(cb).to.be.a('function'); + this.onabort = cb as any; + } + + removeEventListener(type: string, cb: unknown) { + expect(type).to.equal('abort'); + expect(cb).to.be.a('function'); + this.onabort = null; + } + + dispatchEvent(event: Event): boolean { + expect(this.onabort).to.be.a('function'); + this.onabort?.(event); + return true; + } + + dispatchMockAbortEvent(reason?: unknown) { + this.reason = reason; + mockAbortSignal.dispatchEvent(new MockAbortEvent(this)); + } + } + + const mockAbortSignal = new MockAbortSignal(); + + const TestType: GraphQLObjectType = new GraphQLObjectType({ + name: 'TestType', + fields: () => ({ + resolveOnNextTick: { + type: TestType, + resolve: () => resolveOnNextTick({}), + }, + string: { + type: GraphQLString, + args: { + value: { type: new GraphQLNonNull(GraphQLString) }, }, - shouldNotBeResolved: { - type: GraphQLString, - /* c8 ignore next */ - resolve: () => 'This should not be executed!', + resolve: (_, { value }) => value, + }, + abortExecution: { + type: GraphQLString, + resolve: () => { + const abortError = new Error('Custom abort error'); + mockAbortSignal.dispatchMockAbortEvent(abortError); + return 'aborted'; }, - }), - }); + }, + shouldNotBeResolved: { + type: GraphQLString, + /* c8 ignore next */ + resolve: () => 'This should not be executed!', + }, + }), + }); - const schema = new GraphQLSchema({ - query: TestType, - }); + const schema = new GraphQLSchema({ + query: TestType, + }); - const document = parse(` - query { - value1: string(value: "1") + const document = parse(` + query { + value1: string(value: "1") + resolveOnNextTick { + value2: string(value: "2") resolveOnNextTick { - value2: string(value: "2") - resolveOnNextTick { - resolveOnNextTick { - shouldNotBeResolved - } - abortExecution - } - } - alternativeBranch: resolveOnNextTick { - value3: string(value: "3") resolveOnNextTick { shouldNotBeResolved } + abortExecution + } + } + alternativeBranch: resolveOnNextTick { + value3: string(value: "3") + resolveOnNextTick { + shouldNotBeResolved } } - `); - - const abortController = new AbortController(); - const result = await execute({ - schema, - document, - signal: abortController.signal, - }); - - expectJSON(result).toDeepEqual({ - data: { - value1: '1', + } + `); + + const result = await execute({ + schema, + document, + signal: mockAbortSignal, + }); + + expectJSON(result).toDeepEqual({ + data: { + value1: '1', + resolveOnNextTick: { + value2: '2', resolveOnNextTick: { - value2: '2', resolveOnNextTick: { - abortExecution: null, - resolveOnNextTick: null, + shouldNotBeResolved: null, }, - }, - alternativeBranch: { - resolveOnNextTick: null, - value3: '3', + abortExecution: 'aborted', }, }, - errors: [ - { - message: 'Execution aborted.', - path: ['resolveOnNextTick', 'resolveOnNextTick', 'abortExecution'], - locations: [{ line: 10, column: 15 }], - }, - { - message: 'Execution aborted.', - path: ['alternativeBranch', 'resolveOnNextTick'], - locations: [{ line: 15, column: 13 }], - }, - { - message: 'Execution aborted.', - path: [ - 'resolveOnNextTick', - 'resolveOnNextTick', - 'resolveOnNextTick', - ], - locations: [{ line: 7, column: 15 }], + alternativeBranch: { + value3: '3', + resolveOnNextTick: { + shouldNotBeResolved: null, }, - ], - }); + }, + }, + errors: [ + { + message: 'Custom abort error', + path: [ + 'alternativeBranch', + 'resolveOnNextTick', + 'shouldNotBeResolved', + ], + locations: [{ line: 16, column: 13 }], + }, + { + message: 'Custom abort error', + path: [ + 'resolveOnNextTick', + 'resolveOnNextTick', + 'resolveOnNextTick', + 'shouldNotBeResolved', + ], + locations: [{ line: 8, column: 15 }], + }, + ], }); - } + }); }); diff --git a/src/execution/__tests__/mapAsyncIterable-test.ts b/src/execution/__tests__/mapAsyncIterable-test.ts index 0b26ab7327..f00f5c4472 100644 --- a/src/execution/__tests__/mapAsyncIterable-test.ts +++ b/src/execution/__tests__/mapAsyncIterable-test.ts @@ -14,15 +14,22 @@ describe('mapAsyncIterable', () => { yield 3; } - const doubles = mapAsyncIterable(source(), (x) => x + x); + let calledFinishedCallback = false; + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => { calledFinishedCallback = true }, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); expect(await doubles.next()).to.deep.equal({ value: 6, done: false }); + expect(calledFinishedCallback).to.equal(false); expect(await doubles.next()).to.deep.equal({ value: undefined, done: true, }); + expect(calledFinishedCallback).to.equal(true); }); it('maps over async iterable', async () => { @@ -44,15 +51,22 @@ describe('mapAsyncIterable', () => { }, }; - const doubles = mapAsyncIterable(iterable, (x) => x + x); + let calledFinishedCallback = false; + const doubles = mapAsyncIterable( + iterable, + (x) => x + x, + () => { calledFinishedCallback = true }, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); expect(await doubles.next()).to.deep.equal({ value: 6, done: false }); + expect(calledFinishedCallback).to.equal(false); expect(await doubles.next()).to.deep.equal({ value: undefined, done: true, }); + expect(calledFinishedCallback).to.equal(true); }); it('compatible with for-await-of', async () => { @@ -62,12 +76,19 @@ describe('mapAsyncIterable', () => { yield 3; } - const doubles = mapAsyncIterable(source(), (x) => x + x); + let calledFinishedCallback = false; + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => { calledFinishedCallback = true }, + ); const result = []; for await (const x of doubles) { result.push(x); + expect(calledFinishedCallback).to.equal(false); } + expect(calledFinishedCallback).to.equal(true); expect(result).to.deep.equal([2, 4, 6]); }); @@ -78,7 +99,11 @@ describe('mapAsyncIterable', () => { yield 3; } - const doubles = mapAsyncIterable(source(), (x) => Promise.resolve(x + x)); + const doubles = mapAsyncIterable( + source(), + (x) => Promise.resolve(x + x), + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -102,7 +127,11 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable(source(), (x) => x + x); + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -141,7 +170,11 @@ describe('mapAsyncIterable', () => { }, }; - const doubles = mapAsyncIterable(iterable, (x) => x + x); + const doubles = mapAsyncIterable( + iterable, + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -166,7 +199,11 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable(source(), (x) => x + x); + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 'aa', done: false }); expect(await doubles.next()).to.deep.equal({ value: 'bb', done: false }); @@ -205,7 +242,11 @@ describe('mapAsyncIterable', () => { }, }; - const doubles = mapAsyncIterable(iterable, (x) => x + x); + const doubles = mapAsyncIterable( + iterable, + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -228,7 +269,11 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable(source(), (x) => x + x); + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -255,7 +300,11 @@ describe('mapAsyncIterable', () => { throw new Error('Goodbye'); } - const doubles = mapAsyncIterable(source(), (x) => x + x); + const doubles = mapAsyncIterable( + source(), + (x) => x + x, + () => {}, + ); expect(await doubles.next()).to.deep.equal({ value: 'HelloHello', @@ -280,7 +329,7 @@ describe('mapAsyncIterable', () => { } } - const throwOver1 = mapAsyncIterable(source(), mapper); + const throwOver1 = mapAsyncIterable(source(), mapper, () => {}); expect(await throwOver1.next()).to.deep.equal({ value: 1, done: false }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 5e1f2ec65e..e036a9ed26 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,8 +1,3 @@ -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'; @@ -127,11 +122,7 @@ export interface ExecutionContext { subscribeFieldResolver: GraphQLFieldResolver; errors: Array; subsequentPayloads: Set; - abortion: Maybe<{ - passedInAbortSignal: IAbortSignal; - executionAbortController: IAbortController; - executionAbortSignal: IAbortSignal; - }>; + executionController: ExecutionController; } /** @@ -271,7 +262,7 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; - signal?: Maybe; + signal?: AbortSignal | undefined; } const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = @@ -348,28 +339,9 @@ export function experimentalExecuteIncrementally( return executeImpl(exeContext); } -/* c8 ignore start */ -function subscribeToAbortSignal(exeContext: ExecutionContext): () => void { - const { abortion } = exeContext; - if (!abortion) { - return () => null; - } - - const onAbort = () => abortion.executionAbortController.abort(abortion); - abortion.passedInAbortSignal.addEventListener('abort', onAbort); - - return () => { - abortion.passedInAbortSignal.removeEventListener('abort', onAbort); - abortion.executionAbortController.abort(); - }; -} -/* c8 ignore stop */ - function executeImpl( exeContext: ExecutionContext, ): PromiseOrValue { - const unsubscribeFromAbortSignal = subscribeToAbortSignal(exeContext); - // Return a Promise that will eventually resolve to the data described by // The "Response" section of the GraphQL specification. // @@ -386,8 +358,6 @@ function executeImpl( if (isPromise(result)) { return result.then( (data) => { - unsubscribeFromAbortSignal(); - const initialResult = buildResponse(data, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -398,19 +368,18 @@ function executeImpl( subsequentResults: yieldSubsequentPayloads(exeContext), }; } + + exeContext.executionController.abort(); return initialResult; }, (error) => { - unsubscribeFromAbortSignal(); - + exeContext.executionController.abort(); exeContext.errors.push(error); return buildResponse(null, exeContext.errors); }, ); } - unsubscribeFromAbortSignal(); - const initialResult = buildResponse(result, exeContext.errors); if (exeContext.subsequentPayloads.size > 0) { return { @@ -421,9 +390,11 @@ function executeImpl( subsequentResults: yieldSubsequentPayloads(exeContext), }; } + + exeContext.executionController.abort(); return initialResult; } catch (error) { - unsubscribeFromAbortSignal(); + exeContext.executionController.abort(); exeContext.errors.push(error); return buildResponse(null, exeContext.errors); } @@ -478,7 +449,7 @@ export function buildExecutionContext( fieldResolver, typeResolver, subscribeFieldResolver, - signal: passedInAbortSignal, + signal, } = args; // If the schema used for execution is invalid, throw an error. @@ -543,28 +514,36 @@ export function buildExecutionContext( typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, subsequentPayloads: new Set(), + executionController: new ExecutionController(signal), errors: [], - abortion: getContextAbortionEntities(passedInAbortSignal), }; } -/* c8 ignore start */ -function getContextAbortionEntities( - passedInAbortSignal: Maybe, -): ExecutionContext['abortion'] { - if (!passedInAbortSignal) { - return null; +class ExecutionController { + /** For performance reason we can't use `signal.isAborted` so we cache it here */ + isAborted: boolean = false; + + private readonly _passedInAbortSignal: AbortSignal | undefined; + private readonly _abortController: AbortController | undefined = + typeof AbortController !== 'undefined' ? new AbortController() : undefined; + + constructor(signal?: AbortSignal) { + this._passedInAbortSignal = signal; + this._passedInAbortSignal?.addEventListener('abort', this._abortCB); } - const executionAbortController = new AbortController(); + get signal(): AbortSignal | undefined { + return this._abortController?.signal; + } - return { - passedInAbortSignal, - executionAbortController, - executionAbortSignal: executionAbortController.signal, - }; + abort(reason?: unknown) { + this._passedInAbortSignal?.removeEventListener('abort', this._abortCB); + this._abortController?.abort(reason); + this.isAborted = true; + } + + private readonly _abortCB = (event: Event) => this.abort(event.target?.reason); } -/* c8 ignore stop */ function buildPerEventExecutionContext( exeContext: ExecutionContext, @@ -575,6 +554,9 @@ function buildPerEventExecutionContext( rootValue: payload, subsequentPayloads: new Set(), errors: [], + executionController: new ExecutionController( + exeContext.executionController.signal, + ), }; } @@ -765,6 +747,10 @@ function executeField( // Get the resolve function, regardless of if its result is normal or abrupt (error). try { + if (exeContext.executionController.isAborted) { + exeContext.executionController.signal?.throwIfAborted(); + } + // Build a JS object of arguments from the field.arguments AST, using the // variables scope to fulfill any variable references. // TODO: find a way to memoize, in case this field is within a List type. @@ -846,6 +832,7 @@ export function buildResolveInfo( rootValue: exeContext.rootValue, operation: exeContext.operation, variableValues: exeContext.variableValues, + signal: exeContext.executionController.signal, }; } @@ -896,12 +883,6 @@ function completeValue( result: unknown, asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { - /* c8 ignore start */ - if (exeContext.abortion?.executionAbortSignal.aborted) { - throw new GraphQLError('Execution aborted.'); - } - /* c8 ignore stop */ - // If result is an Error, throw a located error. if (result instanceof Error) { throw result; @@ -1690,6 +1671,7 @@ function mapSourceToResponse( // ExperimentalIncrementalExecutionResults when // exeContext.operation is 'subscription'. ) as ExecutionResult, + () => exeContext.executionController.abort(), ); } @@ -1795,6 +1777,10 @@ function executeSubscription( ); try { + if (exeContext.executionController.isAborted) { + exeContext.executionController.signal?.throwIfAborted(); + } + // Implements the "ResolveFieldEventStream" algorithm from GraphQL specification. // It differs from "ResolveFieldValue" due to providing a different `resolveFn`. @@ -2197,6 +2183,7 @@ function yieldSubsequentPayloads( if (!hasNext) { isDone = true; + exeContext.executionController.abort(); } return { @@ -2228,6 +2215,7 @@ function yieldSubsequentPayloads( > { await returnStreamIterators(); isDone = true; + exeContext.executionController.abort(); return { value: undefined, done: true }; }, async throw( @@ -2235,6 +2223,7 @@ function yieldSubsequentPayloads( ): Promise> { await returnStreamIterators(); isDone = true; + exeContext.executionController.abort(); return Promise.reject(error); }, }; diff --git a/src/execution/mapAsyncIterable.ts b/src/execution/mapAsyncIterable.ts index 0f6fd78c2d..a45dda4645 100644 --- a/src/execution/mapAsyncIterable.ts +++ b/src/execution/mapAsyncIterable.ts @@ -6,7 +6,8 @@ import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; */ export function mapAsyncIterable( iterable: AsyncGenerator | AsyncIterable, - callback: (value: T) => PromiseOrValue, + valueCallback: (value: T) => PromiseOrValue, + finallyCallback: () => void, ): AsyncGenerator { const iterator = iterable[Symbol.asyncIterator](); @@ -14,11 +15,12 @@ export function mapAsyncIterable( result: IteratorResult, ): Promise> { if (result.done) { + finallyCallback(); return result; } try { - return { value: await callback(result.value), done: false }; + return { value: await valueCallback(result.value), done: false }; } catch (error) { /* c8 ignore start */ // FIXME: add test case @@ -40,14 +42,16 @@ export function mapAsyncIterable( }, async return(): Promise> { // If iterator.return() does not exist, then type R must be undefined. - return typeof iterator.return === 'function' - ? mapResult(await iterator.return()) - : { value: undefined as any, done: true }; + const result = typeof iterator.return === 'function' + ? await iterator.return() + : { value: undefined as any, done: true } + return mapResult(result); }, async throw(error?: unknown) { if (typeof iterator.throw === 'function') { return mapResult(await iterator.throw(error)); } + finallyCallback(); throw error; }, [Symbol.asyncIterator]() { diff --git a/src/graphql.ts b/src/graphql.ts index 2f9392d54e..84a4fc81ae 100644 --- a/src/graphql.ts +++ b/src/graphql.ts @@ -1,4 +1,3 @@ -import type { IAbortSignal } from './jsutils/AbortController'; import { isPromise } from './jsutils/isPromise.js'; import type { Maybe } from './jsutils/Maybe.js'; import type { PromiseOrValue } from './jsutils/PromiseOrValue.js'; @@ -71,7 +70,7 @@ export interface GraphQLArgs { operationName?: Maybe; fieldResolver?: Maybe>; typeResolver?: Maybe>; - signal?: IAbortSignal; + signal?: AbortSignal; } export function graphql(args: GraphQLArgs): Promise { diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts deleted file mode 100644 index 3657cf6353..0000000000 --- a/src/jsutils/AbortController.ts +++ /dev/null @@ -1,38 +0,0 @@ -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; -} - -// We need to polyfill abort controller in case of Node@14 which doesn't have it -/* c8 ignore start */ -export const AbortController: new () => IAbortController = - // eslint-disable-next-line no-undef - global.AbortController || - // This is a dummy implementation that doesn't actually abort anything - class DummyAbortController 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; - } - }; - -export const hasAbortControllerSupport = - // eslint-disable-next-line no-undef - Boolean(global.AbortController); - -/* c8 ignore stop */ diff --git a/src/type/definition.ts b/src/type/definition.ts index 81488efb39..cb95b3dc65 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -899,6 +899,12 @@ export interface GraphQLResolveInfo { readonly rootValue: unknown; readonly operation: OperationDefinitionNode; readonly variableValues: { [variable: string]: unknown }; + + /** + * Note: signal is undefined only if execution enviroment doesn't support + * AbortController (e.g. node14 without polyfill). + */ + readonly signal: AbortSignal | undefined; } /** From 88cd4246297b1aea0aed9c1ff314c2e754182fe4 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Tue, 7 Feb 2023 07:48:02 +0000 Subject: [PATCH 10/17] fix lint, type check, spellcheck --- .../__tests__/mapAsyncIterable-test.ts | 55 +++++++------------ src/execution/execute.ts | 7 ++- src/type/definition.ts | 4 +- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/src/execution/__tests__/mapAsyncIterable-test.ts b/src/execution/__tests__/mapAsyncIterable-test.ts index f00f5c4472..e45b36029e 100644 --- a/src/execution/__tests__/mapAsyncIterable-test.ts +++ b/src/execution/__tests__/mapAsyncIterable-test.ts @@ -5,6 +5,9 @@ import { expectPromise } from '../../__testUtils__/expectPromise.js'; import { mapAsyncIterable } from '../mapAsyncIterable.js'; +// eslint-disable-next-line @typescript-eslint/no-empty-function +const noop = () => {}; + /* eslint-disable @typescript-eslint/require-await */ describe('mapAsyncIterable', () => { it('maps over async generator', async () => { @@ -18,7 +21,9 @@ describe('mapAsyncIterable', () => { const doubles = mapAsyncIterable( source(), (x) => x + x, - () => { calledFinishedCallback = true }, + () => { + calledFinishedCallback = true; + }, ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); @@ -55,7 +60,9 @@ describe('mapAsyncIterable', () => { const doubles = mapAsyncIterable( iterable, (x) => x + x, - () => { calledFinishedCallback = true }, + () => { + calledFinishedCallback = true; + }, ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); @@ -80,7 +87,9 @@ describe('mapAsyncIterable', () => { const doubles = mapAsyncIterable( source(), (x) => x + x, - () => { calledFinishedCallback = true }, + () => { + calledFinishedCallback = true; + }, ); const result = []; @@ -102,7 +111,7 @@ describe('mapAsyncIterable', () => { const doubles = mapAsyncIterable( source(), (x) => Promise.resolve(x + x), - () => {}, + noop, ); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); @@ -127,11 +136,7 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable( - source(), - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(source(), (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -170,11 +175,7 @@ describe('mapAsyncIterable', () => { }, }; - const doubles = mapAsyncIterable( - iterable, - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(iterable, (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -199,11 +200,7 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable( - source(), - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(source(), (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 'aa', done: false }); expect(await doubles.next()).to.deep.equal({ value: 'bb', done: false }); @@ -242,11 +239,7 @@ describe('mapAsyncIterable', () => { }, }; - const doubles = mapAsyncIterable( - iterable, - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(iterable, (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -269,11 +262,7 @@ describe('mapAsyncIterable', () => { } } - const doubles = mapAsyncIterable( - source(), - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(source(), (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 2, done: false }); expect(await doubles.next()).to.deep.equal({ value: 4, done: false }); @@ -300,11 +289,7 @@ describe('mapAsyncIterable', () => { throw new Error('Goodbye'); } - const doubles = mapAsyncIterable( - source(), - (x) => x + x, - () => {}, - ); + const doubles = mapAsyncIterable(source(), (x) => x + x, noop); expect(await doubles.next()).to.deep.equal({ value: 'HelloHello', @@ -329,7 +314,7 @@ describe('mapAsyncIterable', () => { } } - const throwOver1 = mapAsyncIterable(source(), mapper, () => {}); + const throwOver1 = mapAsyncIterable(source(), mapper, noop); expect(await throwOver1.next()).to.deep.equal({ value: 1, done: false }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index e036a9ed26..f8e1b8a75c 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -542,7 +542,12 @@ class ExecutionController { this.isAborted = true; } - private readonly _abortCB = (event: Event) => this.abort(event.target?.reason); + private readonly _abortCB = (event: Event) => + this.abort( + event.target && 'reason' in event.target + ? event.target.reason + : undefined, + ); } function buildPerEventExecutionContext( diff --git a/src/type/definition.ts b/src/type/definition.ts index cb95b3dc65..55dac7f654 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -901,8 +901,8 @@ export interface GraphQLResolveInfo { readonly variableValues: { [variable: string]: unknown }; /** - * Note: signal is undefined only if execution enviroment doesn't support - * AbortController (e.g. node14 without polyfill). + * Note: signal is undefined only if execution environment doesn't support + * AbortController (e.g. node14 without polyfill). */ readonly signal: AbortSignal | undefined; } From 11a5d321bd45f9b58694e3fcdb0bd17ab25ab471 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Tue, 7 Feb 2023 12:24:27 +0000 Subject: [PATCH 11/17] run prettier --write --- src/__testUtils__/resolveOnNextTick.ts | 4 +++- src/execution/mapAsyncIterable.ts | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/__testUtils__/resolveOnNextTick.ts b/src/__testUtils__/resolveOnNextTick.ts index 48e96a6721..35d065578e 100644 --- a/src/__testUtils__/resolveOnNextTick.ts +++ b/src/__testUtils__/resolveOnNextTick.ts @@ -1,5 +1,7 @@ export function resolveOnNextTick(): Promise; export function resolveOnNextTick(value: T): Promise; -export function resolveOnNextTick(value: unknown = undefined): Promise { +export function resolveOnNextTick( + value: unknown = undefined, +): Promise { return Promise.resolve(value); } diff --git a/src/execution/mapAsyncIterable.ts b/src/execution/mapAsyncIterable.ts index a45dda4645..9616c6c2d0 100644 --- a/src/execution/mapAsyncIterable.ts +++ b/src/execution/mapAsyncIterable.ts @@ -42,9 +42,10 @@ export function mapAsyncIterable( }, async return(): Promise> { // If iterator.return() does not exist, then type R must be undefined. - const result = typeof iterator.return === 'function' - ? await iterator.return() - : { value: undefined as any, done: true } + const result = + typeof iterator.return === 'function' + ? await iterator.return() + : { value: undefined as any, done: true }; return mapResult(result); }, async throw(error?: unknown) { From 8ca80ddfcdd6b93de0b903bc54a97a8d5a5ffe6f Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Fri, 10 Feb 2023 18:12:31 +0000 Subject: [PATCH 12/17] create IAbortSignal, IAbortController types to use instead of globals --- src/execution/__tests__/executor-test.ts | 13 +++++++------ src/execution/execute.ts | 21 ++++++++++++++------- src/graphql.ts | 3 ++- src/jsutils/AbortController.ts | 19 +++++++++++++++++++ src/type/definition.ts | 3 ++- 5 files changed, 44 insertions(+), 15 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 8a45a78ef0..86bca856c3 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 type { IAbortSignal, IEvent } from '../../jsutils/AbortController.js'; import { inspect } from '../../jsutils/inspect.js'; import { Kind } from '../../language/kinds.js'; @@ -1317,7 +1318,7 @@ describe('Execute: Handles basic execution tasks', () => { it('stops execution and throws an error when signal is aborted', async () => { // TODO: use real Event once we can finally drop node14 support - class MockAbortEvent implements Event { + class MockAbortEvent implements IEvent { cancelable = false; bubbles = false; composed = false; @@ -1335,9 +1336,9 @@ describe('Execute: Handles basic execution tasks', () => { CAPTURING_PHASE = 0; NONE = 0; - target: AbortSignal; + target: IAbortSignal; - constructor(abortSignal: AbortSignal) { + constructor(abortSignal: IAbortSignal) { this.target = abortSignal; } @@ -1362,9 +1363,9 @@ describe('Execute: Handles basic execution tasks', () => { }; } - class MockAbortSignal implements AbortSignal { + class MockAbortSignal implements IAbortSignal { aborted: boolean = false; - onabort: ((ev: Event) => any) | null = null; + onabort: ((ev: IEvent) => any) | null = null; reason: unknown; throwIfAborted() { @@ -1386,7 +1387,7 @@ describe('Execute: Handles basic execution tasks', () => { this.onabort = null; } - dispatchEvent(event: Event): boolean { + dispatchEvent(event: IEvent): boolean { expect(this.onabort).to.be.a('function'); this.onabort?.(event); return true; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index f8e1b8a75c..9a55f63fc4 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1,3 +1,8 @@ +import type { + IAbortController, + IAbortSignal, + IEvent, +} from '../jsutils/AbortController.js'; import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; import { isAsyncIterable } from '../jsutils/isAsyncIterable.js'; @@ -262,7 +267,7 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; - signal?: AbortSignal | undefined; + signal?: IAbortSignal | undefined; } const UNEXPECTED_EXPERIMENTAL_DIRECTIVES = @@ -523,16 +528,18 @@ class ExecutionController { /** For performance reason we can't use `signal.isAborted` so we cache it here */ isAborted: boolean = false; - private readonly _passedInAbortSignal: AbortSignal | undefined; - private readonly _abortController: AbortController | undefined = - typeof AbortController !== 'undefined' ? new AbortController() : undefined; + private readonly _passedInAbortSignal: IAbortSignal | undefined; + private readonly _abortController: IAbortController | undefined = + typeof AbortController !== 'undefined' + ? (new AbortController() as IAbortController) + : undefined; - constructor(signal?: AbortSignal) { + constructor(signal?: IAbortSignal) { this._passedInAbortSignal = signal; this._passedInAbortSignal?.addEventListener('abort', this._abortCB); } - get signal(): AbortSignal | undefined { + get signal(): IAbortSignal | undefined { return this._abortController?.signal; } @@ -542,7 +549,7 @@ class ExecutionController { this.isAborted = true; } - private readonly _abortCB = (event: Event) => + private readonly _abortCB = (event: IEvent) => this.abort( event.target && 'reason' in event.target ? event.target.reason diff --git a/src/graphql.ts b/src/graphql.ts index 84a4fc81ae..57853575dd 100644 --- a/src/graphql.ts +++ b/src/graphql.ts @@ -1,3 +1,4 @@ +import type { IAbortSignal } from './jsutils/AbortController.js'; import { isPromise } from './jsutils/isPromise.js'; import type { Maybe } from './jsutils/Maybe.js'; import type { PromiseOrValue } from './jsutils/PromiseOrValue.js'; @@ -70,7 +71,7 @@ export interface GraphQLArgs { operationName?: Maybe; fieldResolver?: Maybe>; typeResolver?: Maybe>; - signal?: AbortSignal; + signal?: IAbortSignal; } export function graphql(args: GraphQLArgs): Promise { diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts new file mode 100644 index 0000000000..8dd5f66650 --- /dev/null +++ b/src/jsutils/AbortController.ts @@ -0,0 +1,19 @@ +export interface IAbortController { + abort: (reason?: unknown) => void; + signal: IAbortSignal; +} + +export interface IEvent { + target: any; +} + +type EventListener = (event: IEvent) => void; + +export interface IAbortSignal { + readonly aborted: boolean; + onabort: ((this: IAbortSignal, ev: IEvent) => any) | null; + readonly reason: any; + throwIfAborted: () => void; + addEventListener: (type: string, listener: EventListener) => void; + removeEventListener: (type: string, listener: EventListener) => void; +} diff --git a/src/type/definition.ts b/src/type/definition.ts index 55dac7f654..d1f65ebf5b 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -1,3 +1,4 @@ +import type { IAbortSignal } from '../jsutils/AbortController.js'; import { devAssert } from '../jsutils/devAssert.js'; import { didYouMean } from '../jsutils/didYouMean.js'; import { identityFunc } from '../jsutils/identityFunc.js'; @@ -904,7 +905,7 @@ export interface GraphQLResolveInfo { * Note: signal is undefined only if execution environment doesn't support * AbortController (e.g. node14 without polyfill). */ - readonly signal: AbortSignal | undefined; + readonly signal: IAbortSignal | undefined; } /** From b15b818684ab58748ba0c1097bb3b5f5151168fd Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Sat, 11 Feb 2023 08:20:07 +0000 Subject: [PATCH 13/17] ignore coverage because of missing AbortController support in node 14 --- src/execution/__tests__/executor-test.ts | 284 +++++++++--------- .../__tests__/mapAsyncIterable-test.ts | 5 +- src/execution/execute.ts | 15 +- src/jsutils/AbortController.ts | 8 +- 4 files changed, 163 insertions(+), 149 deletions(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 86bca856c3..7b8ce7b064 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -1316,126 +1316,128 @@ describe('Execute: Handles basic execution tasks', () => { expect(possibleTypes).to.deep.equal([fooObject]); }); - it('stops execution and throws an error when signal is aborted', async () => { - // TODO: use real Event once we can finally drop node14 support - class MockAbortEvent implements IEvent { - cancelable = false; - bubbles = false; - composed = false; - currentTarget = null; - cancelBubble = false; - defaultPrevented = false; - isTrusted = true; - returnValue = false; - srcElement = null; - type = 'abort'; - eventPhase = 0; - timeStamp = 0; - AT_TARGET = 0; - BUBBLING_PHASE = 0; - CAPTURING_PHASE = 0; - NONE = 0; - - target: IAbortSignal; - - constructor(abortSignal: IAbortSignal) { - this.target = abortSignal; - } + /* c8 ignore start */ + if (typeof AbortController !== 'undefined') { + it('stops execution and throws an error when signal is aborted', async () => { + // TODO: use real Event once we can finally drop node14 support + class MockAbortEvent implements IEvent { + cancelable = false; + bubbles = false; + composed = false; + currentTarget = null; + cancelBubble = false; + defaultPrevented = false; + isTrusted = true; + returnValue = false; + srcElement = null; + type = 'abort'; + eventPhase = 0; + timeStamp = 0; + AT_TARGET = 0; + BUBBLING_PHASE = 0; + CAPTURING_PHASE = 0; + NONE = 0; + + target: IAbortSignal; + + constructor(abortSignal: IAbortSignal) { + this.target = abortSignal; + } - composedPath = () => { - throw new Error('Not mocked!'); - }; + composedPath = () => { + throw new Error('Not mocked!'); + }; - initEvent = () => { - throw new Error('Not mocked!'); - }; + initEvent = () => { + throw new Error('Not mocked!'); + }; - preventDefault = () => { - throw new Error(''); - }; + preventDefault = () => { + throw new Error(''); + }; - stopImmediatePropagation = () => { - throw new Error(''); - }; + stopImmediatePropagation = () => { + throw new Error(''); + }; - stopPropagation = () => { - throw new Error(''); - }; - } + stopPropagation = () => { + throw new Error(''); + }; + } - class MockAbortSignal implements IAbortSignal { - aborted: boolean = false; - onabort: ((ev: IEvent) => any) | null = null; - reason: unknown; + class MockAbortSignal implements IAbortSignal { + aborted: boolean = false; + onabort: ((ev: IEvent) => any) | null = null; + reason: unknown; - throwIfAborted() { - if (this.aborted) { - throw this.reason; + throwIfAborted() { + if (this.aborted) { + throw this.reason; + } } - } - addEventListener(type: string, cb: unknown) { - expect(type).to.equal('abort'); - expect(this.onabort).to.equal(null); - expect(cb).to.be.a('function'); - this.onabort = cb as any; - } + addEventListener(type: string, cb: unknown) { + expect(type).to.equal('abort'); + expect(this.onabort).to.equal(null); + expect(cb).to.be.a('function'); + this.onabort = cb as any; + } - removeEventListener(type: string, cb: unknown) { - expect(type).to.equal('abort'); - expect(cb).to.be.a('function'); - this.onabort = null; - } + removeEventListener(type: string, cb: unknown) { + expect(type).to.equal('abort'); + expect(cb).to.be.a('function'); + this.onabort = null; + } - dispatchEvent(event: IEvent): boolean { - expect(this.onabort).to.be.a('function'); - this.onabort?.(event); - return true; - } + dispatchEvent(event: IEvent): boolean { + expect(this.onabort).to.be.a('function'); + this.onabort?.(event); + return true; + } - dispatchMockAbortEvent(reason?: unknown) { - this.reason = reason; - mockAbortSignal.dispatchEvent(new MockAbortEvent(this)); + dispatchMockAbortEvent(reason?: unknown) { + this.reason = reason; + mockAbortSignal.dispatchEvent(new MockAbortEvent(this)); + } } - } - const mockAbortSignal = new MockAbortSignal(); + const mockAbortSignal = new MockAbortSignal(); - const TestType: GraphQLObjectType = new GraphQLObjectType({ - name: 'TestType', - fields: () => ({ - resolveOnNextTick: { - type: TestType, - resolve: () => resolveOnNextTick({}), - }, - string: { - type: GraphQLString, - args: { - value: { type: new GraphQLNonNull(GraphQLString) }, + const TestType: GraphQLObjectType = new GraphQLObjectType({ + name: 'TestType', + fields: () => ({ + resolveOnNextTick: { + type: TestType, + resolve: () => resolveOnNextTick({}), }, - resolve: (_, { value }) => value, - }, - abortExecution: { - type: GraphQLString, - resolve: () => { - const abortError = new Error('Custom abort error'); - mockAbortSignal.dispatchMockAbortEvent(abortError); - return 'aborted'; + string: { + type: GraphQLString, + args: { + value: { type: new GraphQLNonNull(GraphQLString) }, + }, + resolve: (_, { value }) => value, }, - }, - shouldNotBeResolved: { - type: GraphQLString, - /* c8 ignore next */ - resolve: () => 'This should not be executed!', - }, - }), - }); + abortExecution: { + type: GraphQLString, + resolve: () => { + const abortError = new Error('Custom abort error'); + mockAbortSignal.dispatchMockAbortEvent(abortError); + return 'aborted'; + }, + }, + shouldNotBeResolved: { + type: GraphQLString, + /* c8 ignore next */ + resolve: () => 'This should not be executed!', + }, + }), + }); - const schema = new GraphQLSchema({ - query: TestType, - }); + const schema = new GraphQLSchema({ + query: TestType, + }); - const document = parse(` + const document = parse(` query { value1: string(value: "1") resolveOnNextTick { @@ -1456,52 +1458,54 @@ describe('Execute: Handles basic execution tasks', () => { } `); - const result = await execute({ - schema, - document, - signal: mockAbortSignal, - }); + const result = await execute({ + schema, + document, + signal: mockAbortSignal, + }); - expectJSON(result).toDeepEqual({ - data: { - value1: '1', - resolveOnNextTick: { - value2: '2', + expectJSON(result).toDeepEqual({ + data: { + value1: '1', resolveOnNextTick: { + value2: '2', + resolveOnNextTick: { + resolveOnNextTick: { + shouldNotBeResolved: null, + }, + abortExecution: 'aborted', + }, + }, + alternativeBranch: { + value3: '3', resolveOnNextTick: { shouldNotBeResolved: null, }, - abortExecution: 'aborted', }, }, - alternativeBranch: { - value3: '3', - resolveOnNextTick: { - shouldNotBeResolved: null, + errors: [ + { + message: 'Custom abort error', + path: [ + 'alternativeBranch', + 'resolveOnNextTick', + 'shouldNotBeResolved', + ], + locations: [{ line: 16, column: 13 }], }, - }, - }, - errors: [ - { - message: 'Custom abort error', - path: [ - 'alternativeBranch', - 'resolveOnNextTick', - 'shouldNotBeResolved', - ], - locations: [{ line: 16, column: 13 }], - }, - { - message: 'Custom abort error', - path: [ - 'resolveOnNextTick', - 'resolveOnNextTick', - 'resolveOnNextTick', - 'shouldNotBeResolved', - ], - locations: [{ line: 8, column: 15 }], - }, - ], + { + message: 'Custom abort error', + path: [ + 'resolveOnNextTick', + 'resolveOnNextTick', + 'resolveOnNextTick', + 'shouldNotBeResolved', + ], + locations: [{ line: 8, column: 15 }], + }, + ], + }); }); - }); + } + /* c8 ignore stop */ }); diff --git a/src/execution/__tests__/mapAsyncIterable-test.ts b/src/execution/__tests__/mapAsyncIterable-test.ts index e45b36029e..f3d98af841 100644 --- a/src/execution/__tests__/mapAsyncIterable-test.ts +++ b/src/execution/__tests__/mapAsyncIterable-test.ts @@ -3,10 +3,9 @@ import { describe, it } from 'mocha'; import { expectPromise } from '../../__testUtils__/expectPromise.js'; -import { mapAsyncIterable } from '../mapAsyncIterable.js'; +import { noop } from '../../jsutils/AbortController.js'; -// eslint-disable-next-line @typescript-eslint/no-empty-function -const noop = () => {}; +import { mapAsyncIterable } from '../mapAsyncIterable.js'; /* eslint-disable @typescript-eslint/require-await */ describe('mapAsyncIterable', () => { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 9a55f63fc4..1c6f9cb55b 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -529,10 +529,15 @@ class ExecutionController { isAborted: boolean = false; private readonly _passedInAbortSignal: IAbortSignal | undefined; + + // We don't have AbortController in node 14 so we need to use this hack + // It can be removed once we drop support for node 14 + /* c8 ignore start */ private readonly _abortController: IAbortController | undefined = typeof AbortController !== 'undefined' ? (new AbortController() as IAbortController) : undefined; + /* c8 ignore stop */ constructor(signal?: IAbortSignal) { this._passedInAbortSignal = signal; @@ -550,11 +555,7 @@ class ExecutionController { } private readonly _abortCB = (event: IEvent) => - this.abort( - event.target && 'reason' in event.target - ? event.target.reason - : undefined, - ); + this.abort(event.target.reason); } function buildPerEventExecutionContext( @@ -1789,9 +1790,13 @@ function executeSubscription( ); try { + // Until we have execution cancelling support in Subscriptions, + // ignore test coverage. + /* c8 ignore start */ if (exeContext.executionController.isAborted) { exeContext.executionController.signal?.throwIfAborted(); } + /* c8 ignore stop */ // Implements the "ResolveFieldEventStream" algorithm from GraphQL specification. // It differs from "ResolveFieldValue" due to providing a different `resolveFn`. diff --git a/src/jsutils/AbortController.ts b/src/jsutils/AbortController.ts index 8dd5f66650..6ecf3182c8 100644 --- a/src/jsutils/AbortController.ts +++ b/src/jsutils/AbortController.ts @@ -4,7 +4,7 @@ export interface IAbortController { } export interface IEvent { - target: any; + target: { reason: unknown }; } type EventListener = (event: IEvent) => void; @@ -17,3 +17,9 @@ export interface IAbortSignal { addEventListener: (type: string, listener: EventListener) => void; removeEventListener: (type: string, listener: EventListener) => void; } + +// C8 ignore wasn't working for this file so adding noop function to it, +// to get tests coverage passing +export function noop(): void { + return undefined; +} From fa044f583ec2e40bcbaf16eb3782ed5757d14eea Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Sun, 19 Feb 2023 13:27:15 +0000 Subject: [PATCH 14/17] attempt to fix uncovered lines issue --- src/execution/__tests__/executor-test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 7b8ce7b064..5db4b28057 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -753,6 +753,7 @@ describe('Execute: Handles basic execution tasks', () => { expect(result).to.deep.equal({ data: { second: 'b' } }); }); + /* c8 ignore start */ it('provides error if no operation is provided', () => { const schema = new GraphQLSchema({ query: new GraphQLObjectType({ @@ -770,6 +771,7 @@ describe('Execute: Handles basic execution tasks', () => { errors: [{ message: 'Must provide an operation.' }], }); }); + /* c8 ignore stop */ it('errors if no op name is provided with multiple operations', () => { const schema = new GraphQLSchema({ From 6fa5336c3c23984c7e7bc6eae5ea59283cdfca34 Mon Sep 17 00:00:00 2001 From: "igor.luckenkov" Date: Sun, 19 Feb 2023 13:31:20 +0000 Subject: [PATCH 15/17] Revert "attempt to fix uncovered lines issue" This reverts commit fa044f583ec2e40bcbaf16eb3782ed5757d14eea. --- src/execution/__tests__/executor-test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 5db4b28057..7b8ce7b064 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -753,7 +753,6 @@ describe('Execute: Handles basic execution tasks', () => { expect(result).to.deep.equal({ data: { second: 'b' } }); }); - /* c8 ignore start */ it('provides error if no operation is provided', () => { const schema = new GraphQLSchema({ query: new GraphQLObjectType({ @@ -771,7 +770,6 @@ describe('Execute: Handles basic execution tasks', () => { errors: [{ message: 'Must provide an operation.' }], }); }); - /* c8 ignore stop */ it('errors if no op name is provided with multiple operations', () => { const schema = new GraphQLSchema({ From 1f851b3fb7c5e5d9749b938b06ce937289ca79fa Mon Sep 17 00:00:00 2001 From: Igor Date: Mon, 2 Oct 2023 14:32:41 +0100 Subject: [PATCH 16/17] fix prettier --- src/execution/execute.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 73568728a2..1047c57e21 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -264,11 +264,17 @@ function executeImpl( return data.then( (resolved) => { exeContext.executionController.abort(); - return incrementalPublisher.buildDataResponse(initialResultRecord, resolved) + return incrementalPublisher.buildDataResponse( + initialResultRecord, + resolved, + ); }, (error) => { exeContext.executionController.abort(); - return incrementalPublisher.buildErrorResponse(initialResultRecord, error) + return incrementalPublisher.buildErrorResponse( + initialResultRecord, + error, + ); }, ); } From e781bbcea685d22c8d84af996addfdded6c8bb27 Mon Sep 17 00:00:00 2001 From: Ladislav Louka Date: Mon, 2 Oct 2023 16:10:17 +0200 Subject: [PATCH 17/17] Fix aborted in tests (committing from my account to run the PR checks) --- src/execution/execute.ts | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 1047c57e21..e17dd37e89 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -262,28 +262,15 @@ function executeImpl( const data = executeOperation(exeContext, initialResultRecord); if (isPromise(data)) { return data.then( - (resolved) => { - exeContext.executionController.abort(); - return incrementalPublisher.buildDataResponse( - initialResultRecord, - resolved, - ); - }, - (error) => { - exeContext.executionController.abort(); - return incrementalPublisher.buildErrorResponse( - initialResultRecord, - error, - ); - }, + (resolved) => + incrementalPublisher.buildDataResponse(initialResultRecord, resolved), + (error) => + incrementalPublisher.buildErrorResponse(initialResultRecord, error), ); } - exeContext.executionController.abort(); return incrementalPublisher.buildDataResponse(initialResultRecord, data); } catch (error) { - exeContext.executionController.abort(); - return incrementalPublisher.buildErrorResponse(initialResultRecord, error); } }