Skip to content

Commit 6bdb8b8

Browse files
committed
stop resolvers after execution ends
addresses: #3792
1 parent 8bdda03 commit 6bdb8b8

File tree

2 files changed

+118
-22
lines changed

2 files changed

+118
-22
lines changed

src/execution/__tests__/nonnull-test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { expectJSON } from '../../__testUtils__/expectJSON.js';
5+
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
56

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

@@ -526,6 +527,68 @@ describe('Execute: handles non-nullable types', () => {
526527
});
527528
});
528529

530+
describe('cancellation with null bubbling', () => {
531+
function nestedPromise(n: number): string {
532+
return n > 0 ? `promiseNest { ${nestedPromise(n - 1)} }` : 'promise';
533+
}
534+
535+
it('returns an single error without cancellation', async () => {
536+
const query = `
537+
{
538+
promiseNonNull,
539+
${nestedPromise(4)}
540+
}
541+
`;
542+
543+
const result = await executeQuery(query, throwingData);
544+
expectJSON(result).toDeepEqual({
545+
data: null,
546+
errors: [
547+
// does not include syncNullError because result returns prior to it being added
548+
{
549+
message: 'promiseNonNull',
550+
path: ['promiseNonNull'],
551+
locations: [{ line: 3, column: 11 }],
552+
},
553+
],
554+
});
555+
});
556+
557+
it('stops running despite error', async () => {
558+
const query = `
559+
{
560+
promiseNonNull,
561+
${nestedPromise(10)}
562+
}
563+
`;
564+
565+
let counter = 0;
566+
const rootValue = {
567+
...throwingData,
568+
promiseNest() {
569+
return new Promise((resolve) => {
570+
counter++;
571+
resolve(rootValue);
572+
});
573+
},
574+
};
575+
const result = await executeQuery(query, rootValue);
576+
expectJSON(result).toDeepEqual({
577+
data: null,
578+
errors: [
579+
{
580+
message: 'promiseNonNull',
581+
path: ['promiseNonNull'],
582+
locations: [{ line: 3, column: 11 }],
583+
},
584+
],
585+
});
586+
const counterAtExecutionEnd = counter;
587+
await resolveOnNextTick();
588+
expect(counter).to.equal(counterAtExecutionEnd);
589+
});
590+
});
591+
529592
describe('Handles non-null argument', () => {
530593
const schemaWithNonNullArg = new GraphQLSchema({
531594
query: new GraphQLObjectType({

src/execution/execute.ts

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,13 @@ export interface ExecutionContext {
165165
validatedExecutionArgs: ValidatedExecutionArgs;
166166
errors: Array<GraphQLError> | undefined;
167167
canceller: Canceller;
168+
completed: boolean;
168169
cancellableStreams: Set<CancellableStreamRecord> | undefined;
169170
}
170171

171172
interface IncrementalContext {
172173
errors: Array<GraphQLError> | undefined;
174+
completed: boolean;
173175
deferUsageSet?: DeferUsageSet | undefined;
174176
}
175177

@@ -316,6 +318,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
316318
validatedExecutionArgs,
317319
errors: undefined,
318320
canceller: new Canceller(validatedExecutionArgs.abortSignal),
321+
completed: false,
319322
cancellableStreams: undefined,
320323
};
321324
try {
@@ -366,8 +369,12 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
366369

367370
if (isPromise(graphqlWrappedResult)) {
368371
return graphqlWrappedResult.then(
369-
(resolved) => buildDataResponse(exeContext, resolved),
372+
(resolved) => {
373+
exeContext.completed = true;
374+
return buildDataResponse(exeContext, resolved);
375+
},
370376
(error: unknown) => {
377+
exeContext.completed = true;
371378
exeContext.canceller.unsubscribe();
372379
return {
373380
data: null,
@@ -376,8 +383,10 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
376383
},
377384
);
378385
}
386+
exeContext.completed = true;
379387
return buildDataResponse(exeContext, graphqlWrappedResult);
380388
} catch (error) {
389+
exeContext.completed = true;
381390
exeContext.canceller.unsubscribe();
382391
return { data: null, errors: withError(exeContext.errors, error) };
383392
}
@@ -1754,6 +1763,10 @@ function completeObjectValue(
17541763
incrementalContext: IncrementalContext | undefined,
17551764
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord> | undefined,
17561765
): PromiseOrValue<GraphQLWrappedResult<ObjMap<unknown>>> {
1766+
if ((incrementalContext ?? exeContext).completed) {
1767+
throw new Error('Completed, aborting.');
1768+
}
1769+
17571770
// If there is an isTypeOf predicate function, call it with the
17581771
// current result. If isTypeOf returns false, then raise an error rather
17591772
// than continuing execution.
@@ -2271,6 +2284,7 @@ function collectExecutionGroups(
22712284
groupedFieldSet,
22722285
{
22732286
errors: undefined,
2287+
completed: false,
22742288
deferUsageSet,
22752289
},
22762290
deferMap,
@@ -2330,6 +2344,7 @@ function executeExecutionGroup(
23302344
deferMap,
23312345
);
23322346
} catch (error) {
2347+
incrementalContext.completed = true;
23332348
return {
23342349
pendingExecutionGroup,
23352350
path: pathToArray(path),
@@ -2339,21 +2354,27 @@ function executeExecutionGroup(
23392354

23402355
if (isPromise(result)) {
23412356
return result.then(
2342-
(resolved) =>
2343-
buildCompletedExecutionGroup(
2357+
(resolved) => {
2358+
incrementalContext.completed = true;
2359+
return buildCompletedExecutionGroup(
23442360
incrementalContext.errors,
23452361
pendingExecutionGroup,
23462362
path,
23472363
resolved,
2348-
),
2349-
(error: unknown) => ({
2350-
pendingExecutionGroup,
2351-
path: pathToArray(path),
2352-
errors: withError(incrementalContext.errors, error as GraphQLError),
2353-
}),
2364+
);
2365+
},
2366+
(error: unknown) => {
2367+
incrementalContext.completed = true;
2368+
return {
2369+
pendingExecutionGroup,
2370+
path: pathToArray(path),
2371+
errors: withError(incrementalContext.errors, error as GraphQLError),
2372+
};
2373+
},
23542374
);
23552375
}
23562376

2377+
incrementalContext.completed = true;
23572378
return buildCompletedExecutionGroup(
23582379
incrementalContext.errors,
23592380
pendingExecutionGroup,
@@ -2408,7 +2429,7 @@ function buildSyncStreamItemQueue(
24082429
initialPath,
24092430
initialItem,
24102431
exeContext,
2411-
{ errors: undefined },
2432+
{ errors: undefined, completed: false },
24122433
fieldDetailsList,
24132434
info,
24142435
itemType,
@@ -2439,7 +2460,7 @@ function buildSyncStreamItemQueue(
24392460
itemPath,
24402461
value,
24412462
exeContext,
2442-
{ errors: undefined },
2463+
{ errors: undefined, completed: false },
24432464
fieldDetailsList,
24442465
info,
24452466
itemType,
@@ -2531,7 +2552,7 @@ async function getNextAsyncStreamItemResult(
25312552
itemPath,
25322553
iteration.value,
25332554
exeContext,
2534-
{ errors: undefined },
2555+
{ errors: undefined, completed: false },
25352556
fieldDetailsList,
25362557
info,
25372558
itemType,
@@ -2578,11 +2599,16 @@ function completeStreamItem(
25782599
incrementalContext,
25792600
new Map(),
25802601
).then(
2581-
(resolvedItem) =>
2582-
buildStreamItemResult(incrementalContext.errors, resolvedItem),
2583-
(error: unknown) => ({
2584-
errors: withError(incrementalContext.errors, error as GraphQLError),
2585-
}),
2602+
(resolvedItem) => {
2603+
incrementalContext.completed = true;
2604+
return buildStreamItemResult(incrementalContext.errors, resolvedItem);
2605+
},
2606+
(error: unknown) => {
2607+
incrementalContext.completed = true;
2608+
return {
2609+
errors: withError(incrementalContext.errors, error as GraphQLError),
2610+
};
2611+
},
25862612
);
25872613
}
25882614

@@ -2611,6 +2637,7 @@ function completeStreamItem(
26112637
result = { rawResult: null, incrementalDataRecords: undefined };
26122638
}
26132639
} catch (error) {
2640+
incrementalContext.completed = true;
26142641
return {
26152642
errors: withError(incrementalContext.errors, error),
26162643
};
@@ -2630,14 +2657,20 @@ function completeStreamItem(
26302657
return { rawResult: null, incrementalDataRecords: undefined };
26312658
})
26322659
.then(
2633-
(resolvedItem) =>
2634-
buildStreamItemResult(incrementalContext.errors, resolvedItem),
2635-
(error: unknown) => ({
2636-
errors: withError(incrementalContext.errors, error as GraphQLError),
2637-
}),
2660+
(resolvedItem) => {
2661+
incrementalContext.completed = true;
2662+
return buildStreamItemResult(incrementalContext.errors, resolvedItem);
2663+
},
2664+
(error: unknown) => {
2665+
incrementalContext.completed = true;
2666+
return {
2667+
errors: withError(incrementalContext.errors, error as GraphQLError),
2668+
};
2669+
},
26382670
);
26392671
}
26402672

2673+
incrementalContext.completed = true;
26412674
return buildStreamItemResult(incrementalContext.errors, result);
26422675
}
26432676

0 commit comments

Comments
 (0)