do not use undefined for empty errors array

Author: yaacovCR

src/execution/IncrementalPublisher.ts

@@ -32,7 +32,7 @@ import {
 export function buildIncrementalResponse(
   context: IncrementalPublisherContext,
   result: ObjMap<unknown>,
-  errors: ReadonlyArray<GraphQLError> | undefined,
+  errors: ReadonlyArray<GraphQLError>,
   incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
 ): ExperimentalIncrementalExecutionResults {
   const incrementalPublisher = new IncrementalPublisher(context);
@@ -73,7 +73,7 @@ class IncrementalPublisher {
 
   buildResponse(
     data: ObjMap<unknown>,
-    errors: ReadonlyArray<GraphQLError> | undefined,
+    errors: ReadonlyArray<GraphQLError>,
     incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
   ): ExperimentalIncrementalExecutionResults {
     const newRootNodes = this._incrementalGraph.getNewRootNodes(
@@ -82,10 +82,9 @@ class IncrementalPublisher {
 
     const pending = this._toPendingResults(newRootNodes);
 
-    const initialResult: InitialIncrementalExecutionResult =
-      errors === undefined
-        ? { data, pending, hasNext: true }
-        : { errors, data, pending, hasNext: true };
+    const initialResult: InitialIncrementalExecutionResult = errors.length
+      ? { errors, data, pending, hasNext: true }
+      : { data, pending, hasNext: true };
 
     return {
       initialResult,

src/execution/__tests__/nonnull-test.ts

@@ -544,12 +544,22 @@ describe('Execute: handles non-nullable types', () => {
       expectJSON(result).toDeepEqual({
         data: null,
         errors: [
-          // does not include syncNullError because result returns prior to it being added
           {
             message: 'promiseNonNull',
             path: ['promiseNonNull'],
             locations: [{ line: 3, column: 11 }],
           },
+          {
+            message: 'promise',
+            path: [
+              'promiseNest',
+              'promiseNest',
+              'promiseNest',
+              'promiseNest',
+              'promise',
+            ],
+            locations: [{ line: 4, column: 67 }],
+          },
         ],
       });
     });
@@ -581,6 +591,17 @@ describe('Execute: handles non-nullable types', () => {
             path: ['promiseNonNull'],
             locations: [{ line: 3, column: 11 }],
           },
+          {
+            message: 'Completed, aborting.',
+            path: [
+              'promiseNest',
+              'promiseNest',
+              'promiseNest',
+              'promiseNest',
+              'promiseNest',
+            ],
+            locations: [{ line: 4, column: 67 }],
+          },
         ],
       });
       const counterAtExecutionEnd = counter;

src/execution/execute.ts

@@ -168,15 +168,15 @@ export interface ValidatedExecutionArgs {
 
 export interface ExecutionContext {
   validatedExecutionArgs: ValidatedExecutionArgs;
-  errors: Array<GraphQLError> | undefined;
+  errors: Array<GraphQLError>;
   abortSignalListener: AbortSignalListener | undefined;
   completed: boolean;
   cancellableStreams: Set<CancellableStreamRecord> | undefined;
   errorPropagation: boolean;
 }
 
 interface IncrementalContext {
-  errors: Array<GraphQLError> | undefined;
+  errors: Array<GraphQLError>;
   completed: boolean;
   deferUsageSet?: DeferUsageSet | undefined;
 }
@@ -337,7 +337,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
   const abortSignal = validatedExecutionArgs.abortSignal;
   const exeContext: ExecutionContext = {
     validatedExecutionArgs,
-    errors: undefined,
+    errors: [],
     abortSignalListener: abortSignal
       ? new AbortSignalListener(abortSignal)
       : undefined,
@@ -394,7 +394,7 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
           exeContext.abortSignalListener?.disconnect();
           return {
             data: null,
-            errors: withError(exeContext.errors, error as GraphQLError),
+            errors: addError(exeContext.errors, error as GraphQLError),
           };
         },
       );
@@ -406,15 +406,16 @@ export function experimentalExecuteQueryOrMutationOrSubscriptionEvent(
     // TODO: add test case for synchronous null bubbling to root with cancellation
     /* c8 ignore next */
     exeContext.abortSignalListener?.disconnect();
-    return { data: null, errors: withError(exeContext.errors, error) };
+    return { data: null, errors: addError(exeContext.errors, error) };
   }
 }
 
-function withError(
-  errors: Array<GraphQLError> | undefined,
+function addError(
+  errors: Array<GraphQLError>,
   error: GraphQLError,
 ): ReadonlyArray<GraphQLError> {
-  return errors === undefined ? [error] : [...errors, error];
+  errors.push(error);
+  return errors;
 }
 
 function buildDataResponse(
@@ -425,7 +426,7 @@ function buildDataResponse(
   const errors = exeContext.errors;
   if (incrementalDataRecords === undefined) {
     exeContext.abortSignalListener?.disconnect();
-    return errors !== undefined ? { errors, data } : { data };
+    return errors.length ? { errors, data } : { data };
   }
 
   return buildIncrementalResponse(
@@ -1005,12 +1006,7 @@ function handleFieldError(
   // Otherwise, error protection is applied, logging the error and resolving
   // a null value for this field if one is encountered.
   const context = incrementalContext ?? exeContext;
-  let errors = context.errors;
-  if (errors === undefined) {
-    errors = [];
-    context.errors = errors;
-  }
-  errors.push(error);
+  addError(context.errors, error);
 }
 
 /**
@@ -2397,7 +2393,7 @@ function collectExecutionGroups(
         path,
         groupedFieldSet,
         {
-          errors: undefined,
+          errors: [],
           completed: false,
           deferUsageSet,
         },
@@ -2462,7 +2458,7 @@ function executeExecutionGroup(
     return {
       pendingExecutionGroup,
       path: pathToArray(path),
-      errors: withError(incrementalContext.errors, error),
+      errors: addError(incrementalContext.errors, error),
     };
   }
 
@@ -2482,7 +2478,7 @@ function executeExecutionGroup(
         return {
           pendingExecutionGroup,
           path: pathToArray(path),
-          errors: withError(incrementalContext.errors, error as GraphQLError),
+          errors: addError(incrementalContext.errors, error as GraphQLError),
         };
       },
     );
@@ -2498,7 +2494,7 @@ function executeExecutionGroup(
 }
 
 function buildCompletedExecutionGroup(
-  errors: ReadonlyArray<GraphQLError> | undefined,
+  errors: ReadonlyArray<GraphQLError>,
   pendingExecutionGroup: PendingExecutionGroup,
   path: Path | undefined,
   result: GraphQLWrappedResult<ObjMap<unknown>>,
@@ -2507,7 +2503,7 @@ function buildCompletedExecutionGroup(
   return {
     pendingExecutionGroup,
     path: pathToArray(path),
-    result: errors === undefined ? { data } : { data, errors },
+    result: errors.length ? { errors, data } : { data },
     incrementalDataRecords,
   };
 }
@@ -2543,7 +2539,7 @@ function buildSyncStreamItemQueue(
         initialPath,
         initialItem,
         exeContext,
-        { errors: undefined, completed: false },
+        { errors: [], completed: false },
         fieldDetailsList,
         info,
         itemType,
@@ -2560,7 +2556,7 @@ function buildSyncStreamItemQueue(
       /* c8 ignore next 6 */
       if (currentStreamItem instanceof BoxedPromiseOrValue) {
         const result = currentStreamItem.value;
-        if (!isPromise(result) && result.errors !== undefined) {
+        if (!isPromise(result) && result.item === undefined) {
           break;
         }
       }
@@ -2574,7 +2570,7 @@ function buildSyncStreamItemQueue(
           itemPath,
           value,
           exeContext,
-          { errors: undefined, completed: false },
+          { errors: [], completed: false },
           fieldDetailsList,
           info,
           itemType,
@@ -2666,7 +2662,7 @@ async function getNextAsyncStreamItemResult(
     itemPath,
     iteration.value,
     exeContext,
-    { errors: undefined, completed: false },
+    { errors: [], completed: false },
     fieldDetailsList,
     info,
     itemType,
@@ -2724,7 +2720,7 @@ function completeStreamItem(
       (error: unknown) => {
         incrementalContext.completed = true;
         return {
-          errors: withError(incrementalContext.errors, error as GraphQLError),
+          errors: addError(incrementalContext.errors, error as GraphQLError),
         };
       },
     );
@@ -2757,7 +2753,7 @@ function completeStreamItem(
   } catch (error) {
     incrementalContext.completed = true;
     return {
-      errors: withError(incrementalContext.errors, error),
+      errors: addError(incrementalContext.errors, error),
     };
   }
 
@@ -2782,7 +2778,7 @@ function completeStreamItem(
         (error: unknown) => {
           incrementalContext.completed = true;
           return {
-            errors: withError(incrementalContext.errors, error as GraphQLError),
+            errors: addError(incrementalContext.errors, error as GraphQLError),
           };
         },
       );
@@ -2793,7 +2789,7 @@ function completeStreamItem(
 }
 
 function buildStreamItemResult(
-  errors: ReadonlyArray<GraphQLError> | undefined,
+  errors: ReadonlyArray<GraphQLError>,
   result: GraphQLWrappedResult<unknown>,
 ): StreamItemResult {
   const { rawResult: item, incrementalDataRecords } = result;

src/execution/types.ts

@@ -247,8 +247,8 @@ export function isDeferredFragmentRecord(
 
 export interface StreamItemResult {
   item?: unknown;
+  errors?: ReadonlyArray<GraphQLError>;
   incrementalDataRecords?: ReadonlyArray<IncrementalDataRecord> | undefined;
-  errors?: ReadonlyArray<GraphQLError> | undefined;
 }
 
 export type StreamItemRecord = ThunkIncrementalResult<StreamItemResult>;