Skip to content

Commit 1387e36

Browse files
committed
stop resolvers after execution ends
TODO: add support and tests for stopping within incremental delivery addresses: #3792
1 parent 12a5ec9 commit 1387e36

File tree

3 files changed

+267
-33
lines changed

3 files changed

+267
-33
lines changed

src/execution/__tests__/abort-signal-test.ts

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ describe('Execute: Cancellation', () => {
349349
});
350350
});
351351

352-
it('should stop deferred execution when aborted mid-execution', async () => {
352+
it('should stop deferred execution when aborted prior to initiation of deferred execution', async () => {
353353
const abortController = new AbortController();
354354
const document = parse(`
355355
query {
@@ -358,9 +358,7 @@ describe('Execute: Cancellation', () => {
358358
... on Todo @defer {
359359
text
360360
author {
361-
... on Author @defer {
362-
id
363-
}
361+
id
364362
}
365363
}
366364
}
@@ -428,6 +426,84 @@ describe('Execute: Cancellation', () => {
428426
]);
429427
});
430428

429+
it('should stop deferred execution when aborted within deferred execution', async () => {
430+
const abortController = new AbortController();
431+
const document = parse(`
432+
query {
433+
... on Query @defer {
434+
todo {
435+
id
436+
text
437+
author {
438+
id
439+
}
440+
}
441+
}
442+
}
443+
`);
444+
445+
const resultPromise = complete(
446+
document,
447+
{
448+
todo: async () =>
449+
Promise.resolve({
450+
id: '1',
451+
text: 'hello world',
452+
author: async () =>
453+
/* c8 ignore next 2 */
454+
Promise.resolve({
455+
id: () => expect.fail('Should not be called'),
456+
}),
457+
}),
458+
},
459+
abortController.signal,
460+
);
461+
462+
await resolveOnNextTick();
463+
await resolveOnNextTick();
464+
await resolveOnNextTick();
465+
466+
abortController.abort();
467+
468+
const result = await resultPromise;
469+
470+
expectJSON(result).toDeepEqual([
471+
{
472+
data: {},
473+
pending: [{ id: '0', path: [] }],
474+
hasNext: true,
475+
},
476+
{
477+
incremental: [
478+
{
479+
data: {
480+
todo: {
481+
id: '1',
482+
text: 'hello world',
483+
author: null,
484+
},
485+
},
486+
errors: [
487+
{
488+
locations: [
489+
{
490+
column: 13,
491+
line: 7,
492+
},
493+
],
494+
message: 'This operation was aborted',
495+
path: ['todo', 'author'],
496+
},
497+
],
498+
id: '0',
499+
},
500+
],
501+
completed: [{ id: '0' }],
502+
hasNext: false,
503+
},
504+
]);
505+
});
506+
431507
it('should stop the execution when aborted mid-mutation', async () => {
432508
const abortController = new AbortController();
433509
const document = parse(`

src/execution/__tests__/nonnull-test.ts

Lines changed: 88 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,93 @@ 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+
it('returns both errors if insufficiently nested', async () => {
535+
const query = `
536+
{
537+
promiseNonNull,
538+
${nestedPromise(3)}
539+
}
540+
`;
541+
542+
const result = await executeQuery(query, throwingData);
543+
expectJSON(result).toDeepEqual({
544+
data: null,
545+
errors: [
546+
{
547+
message: 'promise',
548+
path: ['promiseNest', 'promiseNest', 'promiseNest', 'promise'],
549+
locations: [{ line: 4, column: 51 }],
550+
},
551+
{
552+
message: 'promiseNonNull',
553+
path: ['promiseNonNull'],
554+
locations: [{ line: 3, column: 9 }],
555+
},
556+
],
557+
});
558+
});
559+
560+
it('returns only a single error if sufficiently nested', async () => {
561+
const query = `
562+
{
563+
promiseNonNull,
564+
${nestedPromise(4)}
565+
}
566+
`;
567+
568+
const result = await executeQuery(query, throwingData);
569+
expectJSON(result).toDeepEqual({
570+
data: null,
571+
errors: [
572+
// does not include syncNullError because result returns prior to it being added
573+
{
574+
message: 'promiseNonNull',
575+
path: ['promiseNonNull'],
576+
locations: [{ line: 3, column: 11 }],
577+
},
578+
],
579+
});
580+
});
581+
582+
it('keeps running despite error', async () => {
583+
const query = `
584+
{
585+
promiseNonNull,
586+
${nestedPromise(10)}
587+
}
588+
`;
589+
590+
let counter = 0;
591+
const rootValue = {
592+
...throwingData,
593+
promiseNest() {
594+
return new Promise((resolve) => {
595+
counter++;
596+
resolve(rootValue);
597+
});
598+
},
599+
};
600+
const result = await executeQuery(query, rootValue);
601+
expectJSON(result).toDeepEqual({
602+
data: null,
603+
errors: [
604+
{
605+
message: 'promiseNonNull',
606+
path: ['promiseNonNull'],
607+
locations: [{ line: 3, column: 11 }],
608+
},
609+
],
610+
});
611+
const counterAtExecutionEnd = counter;
612+
await resolveOnNextTick();
613+
expect(counter).to.equal(counterAtExecutionEnd);
614+
});
615+
});
616+
529617
describe('Handles non-null argument', () => {
530618
const schemaWithNonNullArg = new GraphQLSchema({
531619
query: new GraphQLObjectType({

0 commit comments

Comments
 (0)