fix(watcher): Improve watcher shutdown precision, add more tests

This commit implements graceful shutdown for the watcher, including
when cancelled via an AbortSignal. It also includes a polyfill for
AbortController, which is necessary on Node v14 (at least without
passing any experimental flags).

Shutdown events happen in logical order, and as long as the caller
does `await stopRunning()`, it's guaranteed the promise will only
resolve after the watch server has terminated completely.

packages/graphql-codegen-cli/src/utils/abort-controller-polyfill.ts

@@ -0,0 +1,105 @@
+import { EventEmitter } from 'events';
+import { debugLog } from './debugging.js';
+
+/**
+ * Node v14 does not have AbortSignal or AbortController, so to safely use it in
+ * another module, you can import it from here.
+ *
+ * Node v14.7+ does have it, but only with flag --experimental-abortcontroller
+ *
+ * We don't actually use AbortController anywhere except in tests, but it
+ * still gets called in watcher.ts, so by polyfilling it we can avoid breaking
+ * existing installations using Node v14 without flag --experimental-abortcontroller,
+ * and we also ensure that tests continue to pass under Node v14 without any new flags.
+ *
+ * This polyfill was adapted (TypeScript-ified) from here:
+ * https://github.com/southpolesteve/node-abort-controller/blob/master/index.js
+ */
+
+class AbortSignalPolyfill implements AbortSignal {
+  eventEmitter: EventEmitter;
+  onabort: EventListener;
+  aborted: boolean;
+  reason: any | undefined;
+
+  constructor() {
+    this.eventEmitter = new EventEmitter();
+    this.onabort = null;
+    this.aborted = false;
+    this.reason = undefined;
+  }
+  toString() {
+    return '[object AbortSignal]';
+  }
+  get [Symbol.toStringTag]() {
+    return 'AbortSignal';
+  }
+  removeEventListener(name, handler) {
+    this.eventEmitter.removeListener(name, handler);
+  }
+  addEventListener(name, handler) {
+    this.eventEmitter.on(name, handler);
+  }
+  // @ts-expect-error No Event type in Node 14
+  dispatchEvent(type: string) {
+    const event = { type, target: this };
+    const handlerName = `on${event.type}`;
+
+    if (typeof this[handlerName] === 'function') this[handlerName](event);
+
+    return this.eventEmitter.emit(event.type, event);
+  }
+  throwIfAborted() {
+    if (this.aborted) {
+      throw this.reason;
+    }
+  }
+  static abort(reason: any) {
+    const controller = new AbortController();
+    controller.abort(reason);
+    return controller.signal;
+  }
+  static timeout(time) {
+    const controller = new AbortController();
+    setTimeout(() => controller.abort(new Error('TimeoutError')), time);
+    return controller.signal;
+  }
+}
+const AbortSignal = global.AbortSignal ?? AbortSignalPolyfill;
+
+class AbortControllerPolyfill implements AbortController {
+  signal: AbortSignal;
+
+  constructor() {
+    debugLog('Using polyfilled AbortController');
+    // @ts-expect-error No Event type in Node 14
+    this.signal = new AbortSignal();
+  }
+  abort(reason?: any) {
+    if (this.signal.aborted) return;
+
+    // @ts-expect-error Not a read only property when polyfilling
+    this.signal.aborted = true;
+
+    if (reason) {
+      // @ts-expect-error Not a read only property when polyfilling
+      this.signal.reason = reason;
+    } else {
+      // @ts-expect-error Not a read only property when polyfilling
+      this.signal.reason = new Error('AbortError');
+    }
+
+    // @ts-expect-error No Event type in Node 14
+    this.signal.dispatchEvent('abort');
+  }
+  toString() {
+    return '[object AbortController]';
+  }
+  get [Symbol.toStringTag]() {
+    return 'AbortController';
+  }
+}
+
+const AbortController = global.AbortController ?? AbortControllerPolyfill;
+
+export { AbortController };

packages/graphql-codegen-cli/src/utils/watcher.ts

@@ -1,4 +1,3 @@
-import { access } from './file-system.js';
 import { join, isAbsolute, resolve, sep } from 'path';
 import { normalizeOutputParam, Types } from '@graphql-codegen/plugin-helpers';
 import type { subscribe } from '@parcel/watcher';
@@ -8,9 +7,11 @@ import logSymbols from 'log-symbols';
 import { executeCodegen } from '../codegen.js';
 import { CodegenContext, loadContext } from '../config.js';
 import { lifecycleHooks } from '../hooks.js';
+import { access } from './file-system.js';
 import { debugLog } from './debugging.js';
 import { getLogger } from './logger.js';
 import { flattenPatternSets, makeGlobalPatternSet, makeLocalPatternSet, makeShouldRebuild } from './patterns.js';
+import { AbortController } from './abort-controller-polyfill.js';
 
 function log(msg: string) {
   // double spaces to inline the message with Listr
@@ -25,9 +26,17 @@ export const createWatcher = (
   initialContext: CodegenContext,
   onNext: (result: Types.FileOutput[]) => Promise<Types.FileOutput[]>
 ): {
-  /** Call this function to stop the running watch server */
-  stopWatching: () => void;
-  /** Promise that will never resolve. To stop it, call stopWatching() */
+  /**
+   * Call this function to stop the running watch server
+   *
+   * @returns Promise that resolves when watcher has terminated ({@link runningWatcher} promise settled)
+   * */
+  stopWatching: () => Promise<void>;
+  /**
+   * Promise that will never resolve as long as the watcher is running. To stop
+   * the watcher, call {@link stopWatching}, which will send a stop signal and
+   * then return a promise that doesn't resolve until `runningWatcher` has resolved.
+   * */
   runningWatcher: Promise<void>;
 } => {
   debugLog(`[Watcher] Starting watcher...`);
@@ -115,35 +124,77 @@ export const createWatcher = (
 
     debugLog(`[Watcher] Started`);
 
-    const shutdown = () => {
+    const shutdown = (
+      /** Optional callback to execute after shutdown has completed its async tasks */
+      afterShutdown?: () => void
+    ) => {
       isShutdown = true;
       debugLog(`[Watcher] Shutting down`);
       log(`Shutting down watch...`);
-      watcherSubscription.unsubscribe();
-      lifecycleHooks(config.hooks).beforeDone();
+
+      const pendingUnsubscribe = watcherSubscription.unsubscribe();
+      const pendingBeforeDoneHook = lifecycleHooks(config.hooks).beforeDone();
+
+      if (afterShutdown && typeof afterShutdown === 'function') {
+        Promise.allSettled([pendingUnsubscribe, pendingBeforeDoneHook]).then(afterShutdown);
+      }
     };
 
-    abortSignal.addEventListener('abort', () => {
-      debugLog(`[Watcher] Got abort signal`);
-      shutdown();
-    });
-    process.once('SIGINT', shutdown);
-    process.once('SIGTERM', shutdown);
+    abortSignal.addEventListener('abort', () => shutdown(abortSignal.reason));
+
+    process.once('SIGINT', () => shutdown());
+    process.once('SIGTERM', () => shutdown());
   };
 
+  // Use an AbortController for shutdown signals
+  // NOTE: This will be polyfilled on Node 14 (or any environment without it defined)
   const abortController = new AbortController();
 
+  /**
+   * Send shutdown signal and return a promise that only resolves after the
+   * runningWatcher has resolved, which only resolved after the shutdown signal has been handled
+   */
+  const stopWatching = async function () {
+    // stopWatching.afterShutdown is lazily set to resolve pendingShutdown promise
+    abortController.abort(stopWatching.afterShutdown);
+
+    // SUBTLE: runningWatcher waits for pendingShutdown before it resolves itself, so
+    // by awaiting it here, we are awaiting both the shutdown handler, and runningWatcher itself
+    await stopWatching.runningWatcher;
+  };
+  stopWatching.afterShutdown = () => {
+    debugLog('Shutdown watcher before it started');
+  };
+  stopWatching.runningWatcher = Promise.resolve();
+
+  /** Promise will resolve after the shutdown() handler completes */
+  const pendingShutdown = new Promise<void>(afterShutdown => {
+    // afterShutdown will be passed to shutdown() handler via abortSignal.reason
+    stopWatching.afterShutdown = afterShutdown;
+  });
+
+  /**
+   * Promise that resolves after the watch server has shutdown, either because
+   * stopWatching() was called or there was an error inside it
+   */
+  stopWatching.runningWatcher = new Promise<void>((resolve, reject) => {
+    executeCodegen(initialContext)
+      .then(onNext, () => Promise.resolve())
+      .then(() => runWatcher(abortController.signal))
+      .catch(err => {
+        watcherSubscription.unsubscribe();
+        reject(err);
+      })
+      .then(() => pendingShutdown)
+      .finally(() => {
+        debugLog('Done watching.');
+        resolve();
+      });
+  });
+
   return {
-    stopWatching: () => abortController.abort(),
-    runningWatcher: new Promise<void>((resolve, reject) => {
-      executeCodegen(initialContext)
-        .then(onNext, () => Promise.resolve())
-        .then(() => runWatcher(abortController.signal))
-        .catch(err => {
-          watcherSubscription.unsubscribe();
-          reject(err);
-        });
-    }),
+    stopWatching,
+    runningWatcher: stopWatching.runningWatcher,
   };
 };
 

packages/graphql-codegen-cli/tests/watcher-helpers.ts

@@ -2,7 +2,7 @@ import type { SubscribeCallback } from '@parcel/watcher';
 import ParcelWatcher from '@parcel/watcher';
 import { CodegenContext } from '../src/config';
 import * as fs from '../src/utils/file-system.js';
-import { createWatcher } from '../src/utils/watcher';
+import { createWatcher } from '../src/utils/watcher.js';
 import { Types } from '@graphql-codegen/plugin-helpers';
 
 /**
@@ -36,7 +36,7 @@ export const setupMockWatcher = async (
   const context = new CodegenContext(contextOpts);
 
   const deferredParcelWatcher = deferMockedParcelWatcher();
-  const { stopper } = createWatcher(context, async _ => Promise.resolve([]));
+  const { stopWatching, runningWatcher } = createWatcher(context, async _ => Promise.resolve([]));
 
   const { dispatchChange, subscribeCallbackSpy, unsubscribeSpy, watchDirectory } = await deferredParcelWatcher;
 
@@ -61,7 +61,14 @@ export const setupMockWatcher = async (
      *
      * This _must be called_ at the end of each test to avoid unhandled promises.
      */
-    stopper,
+    stopWatching,
+    /**
+     * Promise that is pending as long as the watcher is running.
+     *
+     * There should be no need to manually await this, because `await stopWatching()`
+     * will wait for also wait for this same promise to resolve.
+     */
+    runningWatcher,
     /**
      * Asynchronous function for dispatching file change events,
      * _which only resolves after the {@link ParcelWatcher.SubscribeCallback | subscription callback}
@@ -96,28 +103,29 @@ export const setupMockWatcher = async (
  */
 export const setupMockFilesystem = (
   /**
-   * Optionally spy on any {@link fs} functions exported from [file-system.ts](../src/utils/file-system.ts)
+   * Optionally provide the mock implementations for any {@link fs} functions
+   * exported from [file-system.ts](../src/utils/file-system.ts)
    *
    * Default:
    *  * {@link fs.writeFile | `writeFile`}: no-op
    *  * {@link fs.readFile | `readFile`}: return blank string
    *  * {@link fs.access | `access` }: return `void` (indicates file is accessible, since no error is thrown)
    */
-  fsSpies?: Partial<typeof fs>
+  implementations?: Partial<typeof fs>
 ) => {
   const mockedFsSpies = {
     /** Don't write any file */
-    writeFile: fsSpies?.writeFile ?? jest.spyOn(fs, 'writeFile').mockImplementation(),
+    writeFile: jest.spyOn(fs, 'writeFile').mockImplementation(implementations?.writeFile),
     /** Read a blank file */
-    readFile: fsSpies?.readFile ?? jest.spyOn(fs, 'readFile').mockImplementation(async () => ''),
+    readFile: jest.spyOn(fs, 'readFile').mockImplementation(implementations?.readFile ?? (async () => '')),
     /** Always accessible (void means accesible, it throws otherwise) */
-    access: fsSpies?.access ?? jest.spyOn(fs, 'access').mockImplementation(async () => {}),
+    access: jest.spyOn(fs, 'access').mockImplementation(implementations?.access ?? (async () => {})),
   };
 
   return {
     /**
      * The spy functions created for the {@link fs} module, either those provided
-     * by {@link fsSpies} or {@link mockedFsSpies | the defaults}.
+     * by {@link implementations} or {@link mockedFsSpies | the defaults}.
      */
     fsSpies: mockedFsSpies,
   };