Skip to content

Commit 4f03342

Browse files
yaacovCRyaacovCR
authored
fix(execution): finish executors before publishing responses (#4655)
1 parent c8fd614 commit 4f03342

File tree

8 files changed

+743
-1
lines changed

8 files changed

+743
-1
lines changed

‎src/execution/Executor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,9 @@ export class Executor<
366366
buildResponse(
367367
data: ObjMap<unknown> | null,
368368
): ExecutionResult | TAlternativeInitialResponse {
369+
this.finish();
369370
const errors = this.collectedErrors.errors;
370371
const result = errors.length ? { errors, data } : { data };
371-
this.finish();
372372
this.resolverAbortController?.abort();
373373
return result;
374374
}

‎src/execution/__tests__/cancellation-test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,85 @@ describe('Execute: Cancellation', () => {
530530
});
531531
});
532532

533+
it('should stop late sibling object completion after non-null bubbling returns a response', async () => {
534+
const { promise: boomPromise, reject: rejectBoom } =
535+
promiseWithResolvers<string>();
536+
const { promise: sidePromise, resolve: resolveSide } =
537+
promiseWithResolvers<{
538+
value: () => string;
539+
}>();
540+
let lateValueCalls = 0;
541+
const sideType = new GraphQLObjectType({
542+
name: 'LateSide',
543+
fields: {
544+
value: { type: GraphQLString },
545+
},
546+
});
547+
548+
const parentType = new GraphQLObjectType({
549+
name: 'LateParent',
550+
fields: {
551+
boom: {
552+
type: new GraphQLNonNull(GraphQLString),
553+
resolve: () => boomPromise,
554+
},
555+
side: {
556+
type: sideType,
557+
resolve: () => sidePromise,
558+
},
559+
},
560+
});
561+
562+
const bubbleSchema = new GraphQLSchema({
563+
query: new GraphQLObjectType({
564+
name: 'LateQuery',
565+
fields: {
566+
parent: {
567+
type: parentType,
568+
resolve: () => ({}),
569+
},
570+
other: {
571+
type: GraphQLString,
572+
resolve: () => 'ok',
573+
},
574+
},
575+
}),
576+
});
577+
578+
const document = parse('{ parent { boom side { value } } other }');
579+
const resultPromise = execute({ schema: bubbleSchema, document });
580+
581+
rejectBoom(new Error('boom'));
582+
// wait for boom to bubble up
583+
await resolveOnNextTick();
584+
await resolveOnNextTick();
585+
await resolveOnNextTick();
586+
const result = await resultPromise;
587+
resolveSide({
588+
value() {
589+
lateValueCalls += 1;
590+
return 'late value';
591+
},
592+
});
593+
await resolveOnNextTick();
594+
await resolveOnNextTick();
595+
expect(lateValueCalls).to.equal(0);
596+
597+
expectJSON(result).toDeepEqual({
598+
data: {
599+
parent: null,
600+
other: 'ok',
601+
},
602+
errors: [
603+
{
604+
message: 'boom',
605+
locations: [{ line: 1, column: 12 }],
606+
path: ['parent', 'boom'],
607+
},
608+
],
609+
});
610+
});
611+
533612
it('should stop the execution when aborted mid-mutation', async () => {
534613
const abortController = new AbortController();
535614
const document = parse(`

‎src/execution/incremental/IncrementalExecutor.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@ export class IncrementalExecutor<
354354
return super.buildResponse(data);
355355
}
356356

357+
this.finish();
357358
const errors = this.collectedErrors.errors;
358359
invariant(data !== null);
359360
const incrementalPublisher = new IncrementalPublisher();

‎src/execution/incremental/__tests__/defer-test.ts‎

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,6 +2263,124 @@ describe('Execute: defer directive', () => {
22632263
]);
22642264
});
22652265

2266+
it('Stops late initial-path completion before publishing a deferred response', async () => {
2267+
const lateSideType = new GraphQLObjectType({
2268+
name: 'LateInitialSide',
2269+
fields: {
2270+
value: { type: GraphQLString },
2271+
},
2272+
});
2273+
const lateParentType = new GraphQLObjectType({
2274+
name: 'LateInitialParent',
2275+
fields: {
2276+
boom: { type: new GraphQLNonNull(GraphQLString) },
2277+
side: { type: lateSideType },
2278+
},
2279+
});
2280+
const lateGType = new GraphQLObjectType({
2281+
name: 'LateInitialG',
2282+
fields: {
2283+
h: { type: GraphQLString },
2284+
},
2285+
});
2286+
const lateSchema = new GraphQLSchema({
2287+
query: new GraphQLObjectType({
2288+
name: 'LateInitialQuery',
2289+
fields: {
2290+
parent: { type: lateParentType },
2291+
g: { type: lateGType },
2292+
},
2293+
}),
2294+
});
2295+
const document = parse(`
2296+
query {
2297+
parent {
2298+
boom
2299+
side {
2300+
value
2301+
}
2302+
}
2303+
g {
2304+
... @defer {
2305+
h
2306+
}
2307+
}
2308+
}
2309+
`);
2310+
const { promise: boomPromise, reject: rejectBoom } =
2311+
promiseWithResolvers<string>();
2312+
const { promise: sidePromise, resolve: resolveSide } =
2313+
promiseWithResolvers<{
2314+
value: () => string;
2315+
}>();
2316+
let lateValueCalls = 0;
2317+
const resultPromise = experimentalExecuteIncrementally({
2318+
schema: lateSchema,
2319+
document,
2320+
rootValue: {
2321+
parent: {
2322+
boom: () => boomPromise,
2323+
side: () => sidePromise,
2324+
},
2325+
g: {
2326+
h: 'value',
2327+
},
2328+
},
2329+
});
2330+
rejectBoom(new Error('boom'));
2331+
const result = await resultPromise;
2332+
assert('initialResult' in result);
2333+
expectJSON(result.initialResult).toDeepEqual({
2334+
data: {
2335+
parent: null,
2336+
g: {},
2337+
},
2338+
errors: [
2339+
{
2340+
message: 'boom',
2341+
locations: [{ line: 4, column: 11 }],
2342+
path: ['parent', 'boom'],
2343+
},
2344+
],
2345+
pending: [{ id: '0', path: ['g'] }],
2346+
hasNext: true,
2347+
});
2348+
2349+
const iterator = result.subsequentResults[Symbol.asyncIterator]();
2350+
const result2 = await iterator.next();
2351+
expectJSON(result2).toDeepEqual({
2352+
done: false,
2353+
value: {
2354+
incremental: [
2355+
{
2356+
data: {
2357+
h: 'value',
2358+
},
2359+
id: '0',
2360+
},
2361+
],
2362+
completed: [{ id: '0' }],
2363+
hasNext: false,
2364+
},
2365+
});
2366+
2367+
resolveSide({
2368+
value() {
2369+
lateValueCalls += 1;
2370+
return 'late value';
2371+
},
2372+
});
2373+
await resolveOnNextTick();
2374+
await resolveOnNextTick();
2375+
expect(lateValueCalls).to.equal(0);
2376+
2377+
const result3 = await iterator.next();
2378+
expectJSON(result3).toDeepEqual({
2379+
value: undefined,
2380+
done: true,
2381+
});
2382+
});
2383+
22662384
it('Cancels deferred fields when deferred result exhibits null bubbling', async () => {
22672385
const document = parse(`
22682386
query {
@@ -2313,6 +2431,108 @@ describe('Execute: defer directive', () => {
23132431
]);
23142432
});
23152433

2434+
it('Stops late deferred payload completion after deferred null bubbling', async () => {
2435+
const lateSideType = new GraphQLObjectType({
2436+
name: 'LateDeferredSide',
2437+
fields: {
2438+
value: { type: GraphQLString },
2439+
},
2440+
});
2441+
const lateParentType = new GraphQLObjectType({
2442+
name: 'LateDeferredParent',
2443+
fields: {
2444+
side: { type: lateSideType },
2445+
boom: { type: new GraphQLNonNull(GraphQLString) },
2446+
},
2447+
});
2448+
const lateSchema = new GraphQLSchema({
2449+
query: new GraphQLObjectType({
2450+
name: 'LateDeferredQuery',
2451+
fields: {
2452+
parent: { type: lateParentType },
2453+
},
2454+
}),
2455+
});
2456+
const document = parse(`
2457+
query {
2458+
... @defer {
2459+
parent {
2460+
side {
2461+
value
2462+
}
2463+
boom
2464+
}
2465+
}
2466+
}
2467+
`);
2468+
const { promise: boomPromise, reject: rejectBoom } =
2469+
promiseWithResolvers<string>();
2470+
const { promise: sidePromise, resolve: resolveSide } =
2471+
promiseWithResolvers<{
2472+
value: () => string;
2473+
}>();
2474+
let lateValueCalls = 0;
2475+
const resultPromise = experimentalExecuteIncrementally({
2476+
schema: lateSchema,
2477+
document,
2478+
rootValue: {
2479+
parent: {
2480+
side: () => sidePromise,
2481+
boom: () => boomPromise,
2482+
},
2483+
},
2484+
enableEarlyExecution: true,
2485+
});
2486+
rejectBoom(new Error('boom'));
2487+
const result = await resultPromise;
2488+
assert('initialResult' in result);
2489+
expectJSON(result.initialResult).toDeepEqual({
2490+
data: {},
2491+
pending: [{ id: '0', path: [] }],
2492+
hasNext: true,
2493+
});
2494+
const iterator = result.subsequentResults[Symbol.asyncIterator]();
2495+
const result2 = await iterator.next();
2496+
expectJSON(result2).toDeepEqual({
2497+
done: false,
2498+
value: {
2499+
incremental: [
2500+
{
2501+
data: {
2502+
parent: null,
2503+
},
2504+
errors: [
2505+
{
2506+
message: 'boom',
2507+
locations: [{ line: 8, column: 13 }],
2508+
path: ['parent', 'boom'],
2509+
},
2510+
],
2511+
id: '0',
2512+
},
2513+
],
2514+
completed: [{ id: '0' }],
2515+
hasNext: false,
2516+
},
2517+
});
2518+
2519+
resolveSide({
2520+
value() {
2521+
lateValueCalls += 1;
2522+
return 'late value';
2523+
},
2524+
});
2525+
await resolveOnNextTick();
2526+
await resolveOnNextTick();
2527+
expect(lateValueCalls).to.equal(0);
2528+
2529+
const result3 = await iterator.next();
2530+
expectJSON(result3).toDeepEqual({
2531+
value: undefined,
2532+
done: true,
2533+
});
2534+
});
2535+
23162536
it('Deduplicates list fields', async () => {
23172537
const document = parse(`
23182538
query {

0 commit comments

Comments
 (0)