enhance(executor-http): add disposable option (#6325)

* enhance(executor-http): add disposability lazily * Optional * Fix types * TS again
ardatan · Jul 5, 2024 · 9792e80 · 9792e80
1 parent d5baf88
commit 9792e80
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 52 deletions.
diff --git a/.changeset/yellow-weeks-refuse.md b/.changeset/yellow-weeks-refuse.md
@@ -0,0 +1,6 @@
+---
+"@graphql-tools/executor-http": patch
+"@graphql-tools/utils": patch
+---
+
+Make the executor disposable optional
diff --git a/packages/executors/graphql-ws/src/index.ts b/packages/executors/graphql-ws/src/index.ts
@@ -80,7 +80,7 @@ export function buildGraphQLWSExecutor(
     }
     return iterableIterator.next().then(({ value }) => value);
   };
-  const disposableExecutor: DisposableExecutor = executor;
+  const disposableExecutor = executor as DisposableExecutor;
   disposableExecutor[Symbol.asyncDispose] = function disposeWS() {
     return graphqlWSClient.dispose();
   };

diff --git a/packages/executors/http/src/index.ts b/packages/executors/http/src/index.ts
@@ -1,6 +1,7 @@
 import { DocumentNode, GraphQLResolveInfo } from 'graphql';
 import { ValueOrPromise } from 'value-or-promise';
 import {
+  AsyncExecutor,
   createGraphQLError,
   DisposableAsyncExecutor,
   DisposableExecutor,
@@ -9,6 +10,7 @@ import {
   ExecutionResult,
   Executor,
   getOperationASTFromRequest,
+  SyncExecutor,
 } from '@graphql-tools/utils';
 import { fetch as defaultFetch } from '@whatwg-node/fetch';
 import { createFormDataFromVariables } from './createFormDataFromVariables.js';
@@ -85,34 +87,89 @@ export interface HTTPExecutorOptions {
    * Print function for DocumentNode
    */
   print?: (doc: DocumentNode) => string;
+  /**
+   * Enable [Explicit Resource Management](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html#using-declarations-and-explicit-resource-management)
+   * @default false
+   */
+  disposable?: boolean;
 }
 
 export type HeadersConfig = Record<string, string>;
 
 export function buildHTTPExecutor(
-  options?: Omit<HTTPExecutorOptions, 'fetch'> & { fetch: SyncFetchFn },
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: SyncFetchFn;
+    disposable: true;
+  },
 ): DisposableSyncExecutor<any, HTTPExecutorOptions>;
 
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: SyncFetchFn;
+    disposable: false;
+  },
+): SyncExecutor<any, HTTPExecutorOptions>;
+
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch'> & { fetch: SyncFetchFn },
+): SyncExecutor<any, HTTPExecutorOptions>;
+
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: AsyncFetchFn;
+    disposable: true;
+  },
+): DisposableAsyncExecutor<any, HTTPExecutorOptions>;
+
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: AsyncFetchFn;
+    disposable: false;
+  },
+): AsyncExecutor<any, HTTPExecutorOptions>;
+
 export function buildHTTPExecutor(
   options?: Omit<HTTPExecutorOptions, 'fetch'> & { fetch: AsyncFetchFn },
+): AsyncExecutor<any, HTTPExecutorOptions>;
+
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: RegularFetchFn;
+    disposable: true;
+  },
 ): DisposableAsyncExecutor<any, HTTPExecutorOptions>;
 
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & {
+    fetch: RegularFetchFn;
+    disposable: false;
+  },
+): AsyncExecutor<any, HTTPExecutorOptions>;
+
 export function buildHTTPExecutor(
   options?: Omit<HTTPExecutorOptions, 'fetch'> & { fetch: RegularFetchFn },
+): AsyncExecutor<any, HTTPExecutorOptions>;
+
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & { disposable: true },
 ): DisposableAsyncExecutor<any, HTTPExecutorOptions>;
 
+export function buildHTTPExecutor(
+  options?: Omit<HTTPExecutorOptions, 'fetch' | 'disposable'> & { disposable: false },
+): AsyncExecutor<any, HTTPExecutorOptions>;
+
 export function buildHTTPExecutor(
   options?: Omit<HTTPExecutorOptions, 'fetch'>,
-): DisposableAsyncExecutor<any, HTTPExecutorOptions>;
+): AsyncExecutor<any, HTTPExecutorOptions>;
 
 export function buildHTTPExecutor(
   options?: HTTPExecutorOptions,
-): Executor<any, HTTPExecutorOptions> {
+): DisposableExecutor<any, HTTPExecutorOptions> | Executor<any, HTTPExecutorOptions> {
   const printFn = options?.print ?? defaultPrintFn;
-  const disposeCtrl = new AbortController();
-  const executor = (request: ExecutionRequest<any, any, any, HTTPExecutorOptions>) => {
-    if (disposeCtrl.signal.aborted) {
-      throw new Error('Executor was disposed. Aborting execution');
+  let disposeCtrl: AbortController | undefined;
+  const baseExecutor = (request: ExecutionRequest<any, any, any, HTTPExecutorOptions>) => {
+    if (disposeCtrl?.signal.aborted) {
+      return createResultForAbort(disposeCtrl.signal);
     }
     const fetchFn = request.extensions?.fetch ?? options?.fetch ?? defaultFetch;
     let method = request.extensions?.method || options?.method;
@@ -153,17 +210,17 @@ export function buildHTTPExecutor(
 
     const query = printFn(request.document);
 
-    let signal = disposeCtrl.signal;
+    let signal = disposeCtrl?.signal;
     let clearTimeoutFn: VoidFunction = () => {};
     if (options?.timeout) {
       const timeoutCtrl = new AbortController();
       signal = timeoutCtrl.signal;
-      disposeCtrl.signal.addEventListener('abort', clearTimeoutFn);
+      disposeCtrl?.signal.addEventListener('abort', clearTimeoutFn);
       const timeoutId = setTimeout(() => {
         if (!timeoutCtrl.signal.aborted) {
           timeoutCtrl.abort('timeout');
         }
-        disposeCtrl.signal.removeEventListener('abort', clearTimeoutFn);
+        disposeCtrl?.signal.removeEventListener('abort', clearTimeoutFn);
       }, options.timeout);
       clearTimeoutFn = () => {
         clearTimeout(timeoutId);
@@ -349,20 +406,17 @@ export function buildHTTPExecutor(
             ],
           };
         } else if (e.name === 'AbortError' && signal?.reason) {
-          return {
-            errors: [
-              createGraphQLError('The operation was aborted. reason: ' + signal.reason, {
-                extensions: {
-                  requestBody: {
-                    query,
-                    operationName: request.operationName,
-                  },
-                  responseDetails: responseDetailsForError,
-                },
-                originalError: e,
-              }),
-            ],
-          };
+          return createResultForAbort(
+            signal,
+            {
+              requestBody: {
+                query,
+                operationName: request.operationName,
+              },
+              responseDetails: responseDetailsForError,
+            },
+            e,
+          );
         } else if (e.message) {
           return {
             errors: [
@@ -398,11 +452,16 @@ export function buildHTTPExecutor(
       .resolve();
   };
 
+  let executor: Executor = baseExecutor;
+
   if (options?.retry != null) {
-    return function retryExecutor(request: ExecutionRequest) {
+    executor = function retryExecutor(request: ExecutionRequest) {
       let result: ExecutionResult<any> | undefined;
       let attempt = 0;
       function retryAttempt(): Promise<ExecutionResult<any>> | ExecutionResult<any> {
+        if (disposeCtrl?.signal.aborted) {
+          return createResultForAbort(disposeCtrl.signal);
+        }
         attempt++;
         if (attempt > options!.retry!) {
           if (result != null) {
@@ -412,7 +471,7 @@ export function buildHTTPExecutor(
             errors: [createGraphQLError('No response returned from fetch')],
           };
         }
-        return new ValueOrPromise(() => executor(request))
+        return new ValueOrPromise(() => baseExecutor(request))
           .then(res => {
             result = res;
             if (result?.errors?.length) {
@@ -426,17 +485,50 @@ export function buildHTTPExecutor(
     };
   }
 
-  const disposableExecutor: DisposableExecutor = executor;
+  if (!options?.disposable) {
+    disposeCtrl = undefined;
+    return executor;
+  }
 
-  disposableExecutor[Symbol.dispose] = () => {
-    return disposeCtrl.abort(new Error('Executor was disposed. Aborting execution'));
-  };
+  disposeCtrl = new AbortController();
+
+  Object.defineProperties(executor, {
+    [Symbol.dispose]: {
+      get() {
+        return function dispose() {
+          return disposeCtrl!.abort(createAbortErrorReason());
+        };
+      },
+    },
+    [Symbol.asyncDispose]: {
+      get() {
+        return function asyncDispose() {
+          return disposeCtrl!.abort(createAbortErrorReason());
+        };
+      },
+    },
+  });
+
+  return executor;
+}
 
-  disposableExecutor[Symbol.asyncDispose] = () => {
-    return disposeCtrl.abort(new Error('Executor was disposed. Aborting execution'));
-  };
+function createAbortErrorReason() {
+  return new Error('Executor was disposed.');
+}
 
-  return disposableExecutor;
+function createResultForAbort(
+  signal: AbortSignal,
+  extensions?: Record<string, any>,
+  originalError?: Error,
+) {
+  return {
+    errors: [
+      createGraphQLError('The operation was aborted. reason: ' + signal.reason, {
+        extensions,
+        originalError,
+      }),
+    ],
+  };
 }
 
 export { isLiveQueryOperationDefinitionNode };
diff --git a/packages/executors/http/tests/buildHTTPExecutor.test.ts b/packages/executors/http/tests/buildHTTPExecutor.test.ts
@@ -223,6 +223,7 @@ describe('buildHTTPExecutor', () => {
     await new Promise<void>(resolve => server.listen(0, resolve));
     const executor = buildHTTPExecutor({
       endpoint: `http://localhost:${(server.address() as any).port}`,
+      disposable: true,
     });
     const result = executor({
       document: parse(/* GraphQL */ `
@@ -231,29 +232,31 @@ describe('buildHTTPExecutor', () => {
         }
       `),
     });
-    executor[Symbol.dispose]?.();
+    await executor[Symbol.asyncDispose]();
     await expect(result).resolves.toEqual({
       errors: [
-        createGraphQLError(
-          'The operation was aborted. reason: Error: Executor was disposed. Aborting execution',
-        ),
+        createGraphQLError('The operation was aborted. reason: Error: Executor was disposed.'),
       ],
     });
   });
   it('does not allow new requests when the executor is disposed', async () => {
     const executor = buildHTTPExecutor({
       fetch: () => Response.json({ data: { hello: 'world' } }),
+      disposable: true,
     });
     executor[Symbol.dispose]?.();
-    expect(() =>
-      executor({
-        document: parse(/* GraphQL */ `
-          query {
-            hello
-          }
-        `),
-      }),
-    ).toThrow('Executor was disposed. Aborting execution');
+    const result = await executor({
+      document: parse(/* GraphQL */ `
+        query {
+          hello
+        }
+      `),
+    });
+    expect(result).toMatchObject({
+      errors: [
+        createGraphQLError('The operation was aborted. reason: Error: Executor was disposed.'),
+      ],
+    });
   });
   it('should return return GraphqlError instances', async () => {
     const executor = buildHTTPExecutor({

diff --git a/packages/federation/src/gateway.ts b/packages/federation/src/gateway.ts
@@ -14,7 +14,6 @@ import { createDefaultExecutor, SubschemaConfig } from '@graphql-tools/delegate'
 import { buildHTTPExecutor, HTTPExecutorOptions } from '@graphql-tools/executor-http';
 import { stitchSchemas, SubschemaConfigTransform } from '@graphql-tools/stitch';
 import {
-  AsyncExecutor,
   createGraphQLError,
   ExecutionResult,
   Executor,
@@ -40,7 +39,7 @@ export const SubgraphSDLQuery = /* GraphQL */ `
 export async function getSubschemaForFederationWithURL(
   config: HTTPExecutorOptions,
 ): Promise<SubschemaConfig> {
-  const executor = buildHTTPExecutor(config as any) as AsyncExecutor;
+  const executor = buildHTTPExecutor(config);
   const subschemaConfig = await getSubschemaForFederationWithExecutor(executor);
   return {
     batch: true,

diff --git a/packages/utils/src/executor.ts b/packages/utils/src/executor.ts
@@ -42,11 +42,13 @@ export type Executor<TBaseContext = Record<string, any>, TBaseExtensions = Recor
 export type DisposableSyncExecutor<
   TBaseContext = Record<string, any>,
   TBaseExtensions = Record<string, any>,
-> = SyncExecutor<TBaseContext, TBaseExtensions> & { [Symbol.dispose]?: () => void };
+> = SyncExecutor<TBaseContext, TBaseExtensions> & { [Symbol.dispose]: () => void };
 export type DisposableAsyncExecutor<
   TBaseContext = Record<string, any>,
   TBaseExtensions = Record<string, any>,
-> = AsyncExecutor<TBaseContext, TBaseExtensions> & { [Symbol.dispose]?: () => void };
+> = AsyncExecutor<TBaseContext, TBaseExtensions> & {
+  [Symbol.asyncDispose]: () => PromiseLike<void>;
+};
 export type DisposableExecutor<
   TBaseContext = Record<string, any>,
   TBaseExtensions = Record<string, any>,