use single phase of resolver abortSignal cancellation (#4554)

Previously, we used a lazily created abortSignal per resolver so as to
shift triggering the abortSignal for a resolver that returned a streamed
asyncIterable from the original executor to the stream. This is
expensive and difficult to reason about and has been replaced by a
single phase of cancellation when the entire execution finishes.

Note: when executing a subscription, we are still able to cancel
resolver abortSignals after execution of each subscription event.

Additional note: this is labelled "bug fix" rather than "polish" because
it is technically observable.

Author: yaacovCR

src/execution/Executor.ts

@@ -199,33 +199,35 @@ export interface FormattedExecutionResult<
 export class Executor {
   validatedExecutionArgs: ValidatedExecutionArgs;
   finished: boolean;
-  resolverAbortControllers: Map<Path, AbortController>;
   collectedErrors: CollectedErrors;
+  internalAbortController: AbortController;
+  resolverAbortController: AbortController | undefined;
+  sharedResolverAbortSignal: AbortSignal;
 
   constructor(validatedExecutionArgs: ValidatedExecutionArgs) {
     this.validatedExecutionArgs = validatedExecutionArgs;
     this.finished = false;
-    this.resolverAbortControllers = new Map();
     this.collectedErrors = new CollectedErrors();
+    this.internalAbortController = new AbortController();
+
+    this.resolverAbortController = new AbortController();
+    this.sharedResolverAbortSignal = this.resolverAbortController.signal;
   }
 
   executeQueryOrMutationOrSubscriptionEvent(): PromiseOrValue<ExecutionResult> {
-    const validatedExecutionArgs = this.validatedExecutionArgs;
-    const externalAbortSignal = validatedExecutionArgs.externalAbortSignal;
-    let removeAbortListener: (() => void) | undefined;
+    const externalAbortSignal = this.validatedExecutionArgs.externalAbortSignal;
+    let removeExternalAbortListener: (() => void) | undefined;
     if (externalAbortSignal) {
-      if (externalAbortSignal.aborted) {
-        throw new Error(externalAbortSignal.reason);
-      }
+      externalAbortSignal.throwIfAborted();
       const onExternalAbort = () => this.cancel(externalAbortSignal.reason);
-      removeAbortListener = () =>
+      removeExternalAbortListener = () =>
         externalAbortSignal.removeEventListener('abort', onExternalAbort);
       externalAbortSignal.addEventListener('abort', onExternalAbort);
     }
 
     const onFinish = () => {
-      removeAbortListener?.();
       this.finish();
+      removeExternalAbortListener?.();
     };
 
     try {
@@ -236,7 +238,7 @@ export class Executor {
         operation,
         variableValues,
         hideSuggestions,
-      } = validatedExecutionArgs;
+      } = this.validatedExecutionArgs;
 
       const { operation: operationType, selectionSet } = operation;
 
@@ -276,59 +278,38 @@ export class Executor {
             return this.buildResponse(null);
           },
         );
-        return externalAbortSignal
-          ? cancellablePromise(promise, externalAbortSignal)
-          : promise;
+        return cancellablePromise(promise, this.internalAbortController.signal);
       }
       onFinish();
       return this.buildResponse(result);
     } catch (error) {
-      this.collectedErrors.add(error as GraphQLError, undefined);
       onFinish();
+      this.collectedErrors.add(error as GraphQLError, undefined);
       return this.buildResponse(null);
     }
   }
 
   cancel(reason?: unknown): void {
     if (!this.finished) {
-      this.finish(reason);
+      this.finish();
+      this.internalAbortController.abort(reason);
+      this.resolverAbortController?.abort(reason);
     }
   }
 
-  finish(reason?: unknown): void {
+  finish(): void {
     if (!this.finished) {
       this.finished = true;
-      this.triggerResolverAbortSignals(reason);
     }
-  }
-
-  triggerResolverAbortSignals(reason?: unknown): void {
-    const { resolverAbortControllers } = this;
-    const finishReason =
-      reason ?? new Error('Execution has already completed.');
-    for (const abortController of resolverAbortControllers.values()) {
-      abortController.abort(finishReason);
-    }
-  }
-
-  getAbortSignal(path: Path): AbortSignal {
-    const resolverAbortSignal = this.resolverAbortControllers.get(path)?.signal;
-    if (resolverAbortSignal !== undefined) {
-      return resolverAbortSignal;
-    }
-    const abortController = new AbortController();
-    this.resolverAbortControllers.set(path, abortController);
-    if (this.finished) {
-      abortController.abort(new Error('Execution has already completed.'));
-    }
-    return abortController.signal;
+    this.internalAbortController.signal.throwIfAborted();
   }
 
   /**
    * Given a completed execution context and data, build the `{ errors, data }`
    * response defined by the "Response" section of the GraphQL specification.
    */
   buildResponse(data: ObjMap<unknown> | null): ExecutionResult {
+    this.resolverAbortController?.abort();
     const errors = this.collectedErrors.errors;
     return errors.length ? { errors, data } : { data };
   }
@@ -489,7 +470,7 @@ export class Executor {
       toNodes(fieldDetailsList),
       parentType,
       path,
-      () => this.getAbortSignal(path),
+      () => this.sharedResolverAbortSignal,
     );
 
     // Get the resolve function, regardless of if its result is normal or abrupt (error).
@@ -517,7 +498,6 @@ export class Executor {
           info,
           path,
           result,
-          true,
         );
       }
 
@@ -532,22 +512,13 @@ export class Executor {
       if (isPromise(completed)) {
         // Note: we don't rely on a `catch` method, but we do expect "thenable"
         // to take a second callback for the error case.
-        return completed.then(
-          (resolved) => {
-            this.resolverAbortControllers.delete(path);
-            return resolved;
-          },
-          (rawError: unknown) => {
-            this.resolverAbortControllers.delete(path);
-            this.handleFieldError(rawError, returnType, fieldDetailsList, path);
-            return null;
-          },
-        );
+        return completed.then(undefined, (rawError: unknown) => {
+          this.handleFieldError(rawError, returnType, fieldDetailsList, path);
+          return null;
+        });
       }
-      this.resolverAbortControllers.delete(path);
       return completed;
     } catch (rawError) {
-      this.resolverAbortControllers.delete(path);
       this.handleFieldError(rawError, returnType, fieldDetailsList, path);
       return null;
     }
@@ -694,7 +665,6 @@ export class Executor {
     info: GraphQLResolveInfo,
     path: Path,
     result: Promise<unknown>,
-    isFieldValue?: boolean,
   ): Promise<unknown> {
     try {
       const resolved = await result;
@@ -711,14 +681,8 @@ export class Executor {
       if (isPromise(completed)) {
         completed = await completed;
       }
-      if (isFieldValue) {
-        this.resolverAbortControllers.delete(path);
-      }
       return completed;
     } catch (rawError) {
-      if (isFieldValue) {
-        this.resolverAbortControllers.delete(path);
-      }
       this.handleFieldError(rawError, returnType, fieldDetailsList, path);
       return null;
     }