Skip to content

Commit 83da23a

Browse files
authored
fix: cancellation should not cause incremental completion error (#4259)
incremental completion errors are indicated if and only if null bubbling has caused a previously sent response to be null we must instead just give a partial response with errors within the incremental array The GraphQL Tools implementation of this feature has different behavior where the next call will throw if the operation is aborted, but we have opted to give partial responses, and so we must adjust.
1 parent cf1c080 commit 83da23a

File tree

2 files changed

+25
-19
lines changed

2 files changed

+25
-19
lines changed

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ describe('Execute: Cancellation', () => {
100100
errors: [
101101
{
102102
message: 'Aborted',
103-
path: ['todo', 'id'],
104-
locations: [{ line: 4, column: 11 }],
103+
path: ['todo'],
104+
locations: [{ line: 3, column: 9 }],
105105
},
106106
],
107107
});
@@ -149,8 +149,8 @@ describe('Execute: Cancellation', () => {
149149
errors: [
150150
{
151151
message: 'Aborted',
152-
path: ['todo', 'author', 'id'],
153-
locations: [{ line: 6, column: 13 }],
152+
path: ['todo', 'author'],
153+
locations: [{ line: 5, column: 11 }],
154154
},
155155
],
156156
});
@@ -198,8 +198,8 @@ describe('Execute: Cancellation', () => {
198198
errors: [
199199
{
200200
message: 'Aborted',
201-
path: ['todo', 'id'],
202-
locations: [{ line: 4, column: 11 }],
201+
path: ['todo'],
202+
locations: [{ line: 3, column: 9 }],
203203
},
204204
],
205205
});
@@ -257,23 +257,28 @@ describe('Execute: Cancellation', () => {
257257
hasNext: true,
258258
},
259259
{
260-
completed: [
260+
incremental: [
261261
{
262+
data: {
263+
text: 'hello world',
264+
author: null,
265+
},
262266
errors: [
263267
{
264268
locations: [
265269
{
266270
column: 13,
267-
line: 6,
271+
line: 7,
268272
},
269273
],
270274
message: 'Aborted',
271-
path: ['todo', 'text'],
275+
path: ['todo', 'author'],
272276
},
273277
],
274278
id: '0',
275279
},
276280
],
281+
completed: [{ id: '0' }],
277282
hasNext: false,
278283
},
279284
]);

src/execution/execute.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -729,15 +729,6 @@ function executeFields(
729729
try {
730730
for (const [responseName, fieldDetailsList] of groupedFieldSet) {
731731
const fieldPath = addPath(path, responseName, parentType.name);
732-
const abortSignal = exeContext.validatedExecutionArgs.abortSignal;
733-
if (abortSignal?.aborted) {
734-
throw locatedError(
735-
new Error(abortSignal.reason),
736-
toNodes(fieldDetailsList),
737-
pathToArray(fieldPath),
738-
);
739-
}
740-
741732
const result = executeField(
742733
exeContext,
743734
parentType,
@@ -1737,13 +1728,23 @@ function completeObjectValue(
17371728
incrementalContext: IncrementalContext | undefined,
17381729
deferMap: ReadonlyMap<DeferUsage, DeferredFragmentRecord> | undefined,
17391730
): PromiseOrValue<GraphQLWrappedResult<ObjMap<unknown>>> {
1731+
const validatedExecutionArgs = exeContext.validatedExecutionArgs;
1732+
const abortSignal = validatedExecutionArgs.abortSignal;
1733+
if (abortSignal?.aborted) {
1734+
throw locatedError(
1735+
new Error(abortSignal.reason),
1736+
toNodes(fieldDetailsList),
1737+
pathToArray(path),
1738+
);
1739+
}
1740+
17401741
// If there is an isTypeOf predicate function, call it with the
17411742
// current result. If isTypeOf returns false, then raise an error rather
17421743
// than continuing execution.
17431744
if (returnType.isTypeOf) {
17441745
const isTypeOf = returnType.isTypeOf(
17451746
result,
1746-
exeContext.validatedExecutionArgs.contextValue,
1747+
validatedExecutionArgs.contextValue,
17471748
info,
17481749
);
17491750

0 commit comments

Comments
 (0)