add fix for multiple deferred grouped field sets failing

Author: yaacovCR

src/execution/IncrementalPublisher.ts

@@ -508,13 +508,16 @@ class IncrementalPublisher {
     ) {
       for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) {
         const id = deferredFragmentRecord.id;
-        if (id !== undefined) {
-          this._completed.push({
-            id,
-            errors: deferredGroupedFieldSetResult.errors,
-          });
-          this._pending.delete(deferredFragmentRecord);
+        // This can occur if multiple deferred grouped field sets error for a fragment.
+        if (!this._pending.has(deferredFragmentRecord)) {
+          continue;
         }
+        invariant(id !== undefined);
+        this._completed.push({
+          id,
+          errors: deferredGroupedFieldSetResult.errors,
+        });
+        this._pending.delete(deferredFragmentRecord);
       }
       return;
     }
@@ -535,8 +538,12 @@ class IncrementalPublisher {
       // TODO: add test case for this.
       // Presumably, this can occur if an error causes a fragment to be completed early,
       // while an asynchronous deferred grouped field set result is enqueued.
+      // Presumably, this can also occur if multiple deferred fragments are executed
+      // early and share a deferred grouped field set. This could be worked around
+      // another way, such as by checking whether the completedResultQueue already
+      // contains the completedResult prior to pushing.
       /* c8 ignore next 3 */
-      if (id === undefined) {
+      if (!this._pending.has(deferredFragmentRecord)) {
         continue;
       }
       const reconcilableResults = deferredFragmentRecord.reconcilableResults;
@@ -546,6 +553,7 @@ class IncrementalPublisher {
       ) {
         continue;
       }
+      invariant(id !== undefined);
       for (const reconcilableResult of reconcilableResults) {
         if (reconcilableResult.sent) {
           continue;

src/execution/__tests__/defer-test.ts

@@ -1334,6 +1334,152 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
+  it('Handles multiple erroring deferred grouped field sets', async () => {
+    const document = parse(`
+      query {
+        ... @defer {
+          a {
+            b {
+              c {
+                someError: nonNullErrorField
+              }
+            }
+          }
+        }
+        ... @defer {
+          a {
+            b {
+              c {
+                anotherError: nonNullErrorField
+              }
+            }
+          }
+        }
+      }
+    `);
+    const result = await complete(document, {
+      a: {
+        b: { c: { nonNullErrorField: null } },
+      },
+    });
+    expectJSON(result).toDeepEqual([
+      {
+        data: {},
+        pending: [
+          { id: '0', path: [] },
+          { id: '1', path: [] },
+        ],
+        hasNext: true,
+      },
+      {
+        completed: [
+          {
+            id: '0',
+            errors: [
+              {
+                message:
+                  'Cannot return null for non-nullable field c.nonNullErrorField.',
+                locations: [{ line: 7, column: 17 }],
+                path: ['a', 'b', 'c', 'someError'],
+              },
+            ],
+          },
+          {
+            id: '1',
+            errors: [
+              {
+                message:
+                  'Cannot return null for non-nullable field c.nonNullErrorField.',
+                locations: [{ line: 16, column: 17 }],
+                path: ['a', 'b', 'c', 'anotherError'],
+              },
+            ],
+          },
+        ],
+        hasNext: false,
+      },
+    ]);
+  });
+
+  it('Handles multiple erroring deferred grouped field sets for the same fragment', async () => {
+    const document = parse(`
+      query {
+        ... @defer {
+          a {
+            b {
+              someC: c {
+                d: d
+              }
+              anotherC: c {
+                d: d
+              }
+            }
+          }
+        }
+        ... @defer {
+          a {
+            b {
+              someC: c {
+                someError: nonNullErrorField
+              }
+              anotherC: c {
+                anotherError: nonNullErrorField
+              }
+            }
+          }
+        }
+      }
+    `);
+    const result = await complete(document, {
+      a: {
+        b: { c: { d: 'd', nonNullErrorField: null } },
+      },
+    });
+    expectJSON(result).toDeepEqual([
+      {
+        data: {},
+        pending: [
+          { id: '0', path: [] },
+          { id: '1', path: [] },
+        ],
+        hasNext: true,
+      },
+      {
+        incremental: [
+          {
+            data: { a: { b: { someC: {}, anotherC: {} } } },
+            id: '0',
+          },
+          {
+            data: { d: 'd' },
+            id: '0',
+            subPath: ['a', 'b', 'someC'],
+          },
+          {
+            data: { d: 'd' },
+            id: '0',
+            subPath: ['a', 'b', 'anotherC'],
+          },
+        ],
+        completed: [
+          {
+            id: '1',
+            errors: [
+              {
+                message:
+                  'Cannot return null for non-nullable field c.nonNullErrorField.',
+                locations: [{ line: 19, column: 17 }],
+                path: ['a', 'b', 'someC', 'someError'],
+              },
+            ],
+          },
+          { id: '0' },
+        ],
+        hasNext: false,
+      },
+    ]);
+  });
+
   it('filters a payload with a null that cannot be merged', async () => {
     const document = parse(`
       query {