diff --git a/src/execution/IncrementalGraph.ts b/src/execution/IncrementalGraph.ts index 53b60d2b32..b1bc302299 100644 --- a/src/execution/IncrementalGraph.ts +++ b/src/execution/IncrementalGraph.ts @@ -132,14 +132,8 @@ export class IncrementalGraph { return { newRootNodes, successfulExecutionGroups }; } - removeDeferredFragment( - deferredFragmentRecord: DeferredFragmentRecord, - ): boolean { - if (!this._rootNodes.has(deferredFragmentRecord)) { - return false; - } + removeDeferredFragment(deferredFragmentRecord: DeferredFragmentRecord): void { this._rootNodes.delete(deferredFragmentRecord); - return true; } removeStream(streamRecord: StreamRecord): void { @@ -219,7 +213,10 @@ export class IncrementalGraph { deferredFragmentRecord: DeferredFragmentRecord, initialResultChildren: Set | undefined, ): void { - if (this._rootNodes.has(deferredFragmentRecord)) { + if ( + deferredFragmentRecord.failed || + this._rootNodes.has(deferredFragmentRecord) + ) { return; } const parent = deferredFragmentRecord.parent; diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts index b826a9b9c5..ddb4acc5b2 100644 --- a/src/execution/IncrementalPublisher.ts +++ b/src/execution/IncrementalPublisher.ts @@ -228,13 +228,13 @@ class IncrementalPublisher { if (isFailedExecutionGroup(completedExecutionGroup)) { for (const deferredFragmentRecord of completedExecutionGroup .pendingExecutionGroup.deferredFragmentRecords) { - const id = deferredFragmentRecord.id; - if ( - !this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord) - ) { - // This can occur if multiple deferred grouped field sets error for a fragment. + const failed = deferredFragmentRecord.failed; + if (failed) { continue; } + deferredFragmentRecord.failed = true; + const id = deferredFragmentRecord.id; + this._incrementalGraph.removeDeferredFragment(deferredFragmentRecord); invariant(id !== undefined); context.completed.push({ id, diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts index 97dcfeceb6..55f63c426a 100644 --- a/src/execution/__tests__/defer-test.ts +++ b/src/execution/__tests__/defer-test.ts @@ -99,6 +99,7 @@ const a = new GraphQLObjectType({ fields: { b: { type: b }, someField: { type: GraphQLString }, + nonNullErrorField: { type: new GraphQLNonNull(GraphQLString) }, }, name: 'a', }); @@ -1688,6 +1689,83 @@ describe('Execute: defer directive', () => { ]); }); + it('Nulls cross defer boundaries, failed fragment with slower shared child execution groups', async () => { + const document = parse(` + query { + ... @defer { + a { + someField + nonNullErrorField + b { + c { + d + } + } + } + } + a { + ... @defer { + someField + b { + e { + f + } + } + } + } + } + `); + const result = await complete(document, { + a: { + b: { c: { d: 'd' }, e: { f: 'f' } }, + someField: Promise.resolve('someField'), + }, + }); + expectJSON(result).toDeepEqual([ + { + data: { + a: {}, + }, + pending: [ + { id: '0', path: [] }, + { id: '1', path: ['a'] }, + ], + hasNext: true, + }, + { + completed: [ + { + id: '0', + errors: [ + { + message: + 'Cannot return null for non-nullable field a.nonNullErrorField.', + locations: [{ line: 6, column: 13 }], + path: ['a', 'nonNullErrorField'], + }, + ], + }, + ], + hasNext: true, + }, + { + incremental: [ + { + data: { b: {}, someField: 'someField' }, + id: '1', + }, + { + data: { e: { f: 'f' } }, + id: '1', + subPath: ['b'], + }, + ], + completed: [{ id: '1' }], + hasNext: false, + }, + ]); + }); + it('Handles multiple erroring deferred grouped field sets', async () => { const document = parse(` query { diff --git a/src/execution/types.ts b/src/execution/types.ts index 87d47e3efc..afeaeaa5d1 100644 --- a/src/execution/types.ts +++ b/src/execution/types.ts @@ -224,6 +224,7 @@ export class DeferredFragmentRecord { pendingExecutionGroups: Set; successfulExecutionGroups: Set; children: Set; + failed: true | undefined; constructor( path: Path | undefined,