Skip to content

Commit

Permalink
stop resolvers after execution ends
Browse files Browse the repository at this point in the history 
addresses: #3792
  • Loading branch information
@yaacovCR
yaacovCR committed Oct 30, 2024
1 parent 8bdda03 commit c43b695
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 23 deletions.
63 changes: 63 additions & 0 deletions src/execution/__tests__/nonnull-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import { describe, it } from 'mocha';

import { expectJSON } from '../../__testUtils__/expectJSON.js';
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';

import type { PromiseOrValue } from '../../jsutils/PromiseOrValue.js';

Expand Down Expand Up @@ -526,6 +527,68 @@ describe('Execute: handles non-nullable types', () => {
});
});

describe('cancellation with null bubbling', () => {
function nestedPromise(n: number): string {
return n > 0 ? `promiseNest { ${nestedPromise(n - 1)} }` : 'promise';
}

it('returns an single error without cancellation', async () => {
const query = `
{
promiseNonNull,
${nestedPromise(4)}
}
`;

const result = await executeQuery(query, throwingData);
expectJSON(result).toDeepEqual({
data: null,
errors: [
// does not include syncNullError because result returns prior to it being added
{
message: 'promiseNonNull',
path: ['promiseNonNull'],
locations: [{ line: 3, column: 11 }],
},
],
});
});

it('stops running despite error', async () => {
const query = `
{
promiseNonNull,
${nestedPromise(10)}
}
`;

let counter = 0;
const rootValue = {
...throwingData,
promiseNest() {
return new Promise((resolve) => {
counter++;
resolve(rootValue);
});
},
};
const result = await executeQuery(query, rootValue);
expectJSON(result).toDeepEqual({
data: null,
errors: [
{
message: 'promiseNonNull',
path: ['promiseNonNull'],
locations: [{ line: 3, column: 11 }],
},
],
});
const counterAtExecutionEnd = counter;
await resolveOnNextTick();
expect(counter).to.equal(counterAtExecutionEnd);
});
});

describe('Handles non-null argument', () => {
const schemaWithNonNullArg = new GraphQLSchema({
query: new GraphQLObjectType({
Expand Down
75 changes: 52 additions & 23 deletions src/execution/execute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,13 @@ export interface ExecutionContext {
validatedExecutionArgs: ValidatedExecutionArgs;
errors: Array<GraphQLError> | undefined;
canceller: Canceller;
completed: boolean;
cancellableStreams: Set<CancellableStreamRecord> | undefined;
}

interface IncrementalContext {
errors: Array<GraphQLError> | undefined;
completed: boolean;
deferUsageSet?: DeferUsageSet | undefined;
}

Expand Down Expand Up @@ -316,6 +318,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
validatedExecutionArgs,
errors: undefined,
canceller: new Canceller(validatedExecutionArgs.abortSignal),
completed: false,
cancellableStreams: undefined,
};
try {
Expand Down Expand Up @@ -366,8 +369,12 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(

if (isPromise(graphqlWrappedResult)) {
return graphqlWrappedResult.then(
(resolved) => buildDataResponse(exeContext, resolved),
(resolved) => {
exeContext.completed = true;
return buildDataResponse(exeContext, resolved);
},
(error: unknown) => {
exeContext.completed = true;
exeContext.canceller.unsubscribe();
return {
data: null,
Expand All @@ -376,8 +383,10 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
},
);
}
exeContext.completed = true;
return buildDataResponse(exeContext, graphqlWrappedResult);
} catch (error) {
exeContext.completed = true;
exeContext.canceller.unsubscribe();
return { data: null, errors: withError(exeContext.errors, error) };
}
Expand Down Expand Up @@ -825,7 +834,7 @@ function executeField(
validatedExecutionArgs;
const fieldName = fieldDetailsList[0].node.name.value;
const fieldDef = schema.getField(parentType, fieldName);
if (!fieldDef) {
if (!fieldDef || (incrementalContext ?? exeContext).completed) {
return;
}

Expand Down Expand Up @@ -2271,6 +2280,7 @@ function collectExecutionGroups(
groupedFieldSet,
{
errors: undefined,
completed: false,
deferUsageSet,
},
deferMap,
Expand Down Expand Up @@ -2330,6 +2340,7 @@ function executeExecutionGroup(
deferMap,
);
} catch (error) {
incrementalContext.completed = true;
return {
pendingExecutionGroup,
path: pathToArray(path),
Expand All @@ -2339,21 +2350,27 @@ function executeExecutionGroup(

if (isPromise(result)) {
return result.then(
(resolved) =>
buildCompletedExecutionGroup(
(resolved) => {
incrementalContext.completed = true;
return buildCompletedExecutionGroup(
incrementalContext.errors,
pendingExecutionGroup,
path,
resolved,
),
(error: unknown) => ({
pendingExecutionGroup,
path: pathToArray(path),
errors: withError(incrementalContext.errors, error as GraphQLError),
}),
);
},
(error: unknown) => {
incrementalContext.completed = true;
return {
pendingExecutionGroup,
path: pathToArray(path),
errors: withError(incrementalContext.errors, error as GraphQLError),
};
},
);
}

incrementalContext.completed = true;
return buildCompletedExecutionGroup(
incrementalContext.errors,
pendingExecutionGroup,
Expand Down Expand Up @@ -2408,7 +2425,7 @@ function buildSyncStreamItemQueue(
initialPath,
initialItem,
exeContext,
{ errors: undefined },
{ errors: undefined, completed: false },
fieldDetailsList,
info,
itemType,
Expand Down Expand Up @@ -2439,7 +2456,7 @@ function buildSyncStreamItemQueue(
itemPath,
value,
exeContext,
{ errors: undefined },
{ errors: undefined, completed: false },
fieldDetailsList,
info,
itemType,
Expand Down Expand Up @@ -2531,7 +2548,7 @@ async function getNextAsyncStreamItemResult(
itemPath,
iteration.value,
exeContext,
{ errors: undefined },
{ errors: undefined, completed: false },
fieldDetailsList,
info,
itemType,
Expand Down Expand Up @@ -2578,11 +2595,16 @@ function completeStreamItem(
incrementalContext,
new Map(),
).then(
(resolvedItem) =>
buildStreamItemResult(incrementalContext.errors, resolvedItem),
(error: unknown) => ({
errors: withError(incrementalContext.errors, error as GraphQLError),
}),
(resolvedItem) => {
incrementalContext.completed = true;
return buildStreamItemResult(incrementalContext.errors, resolvedItem);
},
(error: unknown) => {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error as GraphQLError),
};
},
);
}

Expand Down Expand Up @@ -2611,6 +2633,7 @@ function completeStreamItem(
result = { rawResult: null, incrementalDataRecords: undefined };
}
} catch (error) {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error),
};
Expand All @@ -2630,14 +2653,20 @@ function completeStreamItem(
return { rawResult: null, incrementalDataRecords: undefined };
})
.then(
(resolvedItem) =>
buildStreamItemResult(incrementalContext.errors, resolvedItem),
(error: unknown) => ({
errors: withError(incrementalContext.errors, error as GraphQLError),
}),
(resolvedItem) => {
incrementalContext.completed = true;
return buildStreamItemResult(incrementalContext.errors, resolvedItem);
},
(error: unknown) => {
incrementalContext.completed = true;
return {
errors: withError(incrementalContext.errors, error as GraphQLError),
};
},
);
}

incrementalContext.completed = true;
return buildStreamItemResult(incrementalContext.errors, result);
}

Expand Down

0 comments on commit c43b695

Please sign in to comment.