Add serverWillStop lifecycle hook; call stop() on signals by default

Fixes #4273.

This PR adds a serverWillStop plugin lifecycle hook.  The `serverWillStop` hook
is on an object optionally returned from a `serverWillStart` hook, similar to
`executionDidStart`/`executionDidEnd`.

ApolloServerPluginOperationRegistry uses this to stop its agent.

The code that installs SIGINT and SIGTERM handlers unless disabled with
`handleSignals: false` is hoisted from EngineReportingAgent to ApolloServer
itself; `handleSignals` is added as a new ApolloServer option. The only effect
on existing code is that on one of these signals, any SubscriptionServer and
ApolloGateway will be stopped in addition to any EngineReportingAgent.

Author: glasser

docs/source/api/apollo-server.md

@@ -163,6 +163,14 @@ new ApolloServer({
   size of 30MiB, which is generally sufficient unless the server is processing
   a high number of unique operations.
 
+* `handleSignals`: `boolean`
+
+By default, ApolloServer listens for the `SIGINT` and `SIGTERM` signals and calls `await this.stop()` on
+itself when it is received, and then re-sends the signal to itself. Set this to false to disable
+this behavior. You can manually invoke `stop()` in other contexts if you'd like. Note that `stop()` does
+not run synchronously so it cannot work usefully in an `exit` handler.
+
+
 #### Returns
 
 `ApolloServer`
@@ -447,11 +455,8 @@ addMockFunctionsToSchema({
 
 *  `handleSignals`: boolean
 
-   By default, EngineReportingAgent listens for the 'SIGINT' and 'SIGTERM'
-   signals, stops, sends a final report, and re-sends the signal to
-   itself. Set this to false to disable. You can manually invoke 'stop()' and
-   'sendReport()' on other signals if you'd like. Note that 'sendReport()'
-   does not run synchronously so it cannot work usefully in an 'exit' handler.
+  For backwards compatibility only; specifying `new ApolloServer({engine: {handleSignals: false}})` is
+  equivalent to specifying `new ApolloServer({handleSignals: false})`.
 
 *  `rewriteError`: (err: GraphQLError) => GraphQLError | null
 

packages/apollo-engine-reporting/src/agent.ts

@@ -290,11 +290,8 @@ export interface EngineReportingOptions<TContext> {
    */
   privateHeaders?: Array<String> | boolean;
   /**
-   * By default, EngineReportingAgent listens for the 'SIGINT' and 'SIGTERM'
-   * signals, stops, sends a final report, and re-sends the signal to
-   * itself. Set this to false to disable. You can manually invoke 'stop()' and
-   * 'sendReport()' on other signals if you'd like. Note that 'sendReport()'
-   * does not run synchronously so it cannot work usefully in an 'exit' handler.
+   * For backwards compatibility only; specifying `new ApolloServer({engine: {handleSignals: false}})` is
+   * equivalent to specifying `new ApolloServer({handleSignals: false})`.
    */
   handleSignals?: boolean;
   /**
@@ -445,8 +442,6 @@ export class EngineReportingAgent<TContext = any> {
   private stopped: boolean = false;
   private signatureCache: InMemoryLRUCache<string>;
 
-  private signalHandlers = new Map<NodeJS.Signals, NodeJS.SignalsListener>();
-
   private currentSchemaReporter?: SchemaReporter;
   private readonly bootId: string;
   private lastSeenExecutableSchemaToId?: {
@@ -529,21 +524,6 @@ export class EngineReportingAgent<TContext = any> {
       );
     }
 
-    if (this.options.handleSignals !== false) {
-      const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM'];
-      signals.forEach(signal => {
-        // Note: Node only started sending signal names to signal events with
-        // Node v10 so we can't use that feature here.
-        const handler: NodeJS.SignalsListener = async () => {
-          this.stop();
-          await this.sendAllReportsAndReportErrors();
-          process.kill(process.pid, signal);
-        };
-        process.once(signal, handler);
-        this.signalHandlers.set(signal, handler);
-      });
-    }
-
     if (this.options.endpointUrl) {
       this.logger.warn(
         '[deprecated] The `endpointUrl` option within `engine` has been renamed to `tracesEndpointUrl`.',
@@ -847,11 +827,6 @@ export class EngineReportingAgent<TContext = any> {
   // size, and stop buffering new traces. You may still manually send a last
   // report by calling sendReport().
   public stop() {
-    // Clean up signal handlers so they don't accrue indefinitely.
-    this.signalHandlers.forEach((handler, signal) => {
-      process.removeListener(signal, handler);
-    });
-
     if (this.reportTimer) {
       clearInterval(this.reportTimer);
       this.reportTimer = undefined;
@@ -930,7 +905,7 @@ export class EngineReportingAgent<TContext = any> {
     return generatedSignature;
   }
 
-  private async sendAllReportsAndReportErrors(): Promise<void> {
+  public async sendAllReportsAndReportErrors(): Promise<void> {
     await Promise.all(
       Object.keys(this.reportDataByExecutableSchemaId).map(executableSchemaId =>
         this.sendReportAndReportErrors(executableSchemaId),

packages/apollo-server-core/src/ApolloServer.ts

@@ -27,6 +27,7 @@ import {
 import {
   ApolloServerPlugin,
   GraphQLServiceContext,
+  GraphQLServerListener,
 } from 'apollo-server-plugin-base';
 import runtimeSupportsUploads from './utils/runtimeSupportsUploads';
 
@@ -72,13 +73,14 @@ import {
 import { Headers } from 'apollo-server-env';
 import { buildServiceDefinition } from '@apollographql/apollo-tools';
 import { plugin as pluginTracing } from "apollo-tracing";
-import { Logger, SchemaHash } from "apollo-server-types";
+import { Logger, SchemaHash, ValueOrPromise } from "apollo-server-types";
 import {
   plugin as pluginCacheControl,
   CacheControlExtensionOptions,
 } from 'apollo-cache-control';
 import { getEngineApiKey, getEngineGraphVariant } from "apollo-engine-reporting/dist/agent";
 import { cloneObject } from "./runHttpQuery";
+import { Dispatcher } from './utils/dispatcher';
 
 const NoIntrospection = (context: ValidationContext) => ({
   Field(node: FieldDefinitionNode) {
@@ -145,7 +147,7 @@ export class ApolloServerBase {
   private config: Config;
   /** @deprecated: This is undefined for servers operating as gateways, and will be removed in a future release **/
   protected schema?: GraphQLSchema;
-  private toDispose = new Set<() => void>();
+  private toDispose = new Set<() => ValueOrPromise<void>>();
   private experimental_approximateDocumentStoreMiB:
     Config['experimental_approximateDocumentStoreMiB'];
 
@@ -173,6 +175,7 @@ export class ApolloServerBase {
       gateway,
       cacheControl,
       experimental_approximateDocumentStoreMiB,
+      handleSignals,
       ...requestOptions
     } = config;
 
@@ -381,6 +384,26 @@ export class ApolloServerBase {
     // is populated accordingly.
     this.ensurePluginInstantiation(plugins);
 
+    if (
+      handleSignals !== false &&
+      // For backwards compatibility.
+      (typeof this.config.engine !== 'object' ||
+        this.config.engine.handleSignals !== false)
+    ) {
+      const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM'];
+      signals.forEach((signal) => {
+        // Note: Node only started sending signal names to signal events with
+        // Node v10 so we can't use that feature here.
+        const handler: NodeJS.SignalsListener = async () => {
+          await this.stop();
+          process.kill(process.pid, signal);
+        };
+        process.once(signal, handler);
+        this.toDispose.add(() => {
+          process.removeListener(signal, handler);
+        });
+      });
+    }
   }
 
   // used by integrations to synchronize the path with subscriptions, some
@@ -581,24 +604,33 @@ export class ApolloServerBase {
     if (this.requestOptions.persistedQueries?.cache) {
       service.persistedQueries = {
         cache: this.requestOptions.persistedQueries.cache,
-      }
+      };
     }
 
-    await Promise.all(
-      this.plugins.map(
-        plugin =>
-          plugin.serverWillStart &&
-          plugin.serverWillStart(service),
-      ),
+    const serverListeners = (
+      await Promise.all(
+        this.plugins.map(
+          (plugin) => plugin.serverWillStart && plugin.serverWillStart(service),
+        ),
+      )
+    ).filter(
+      (maybeServerListener): maybeServerListener is GraphQLServerListener =>
+        typeof maybeServerListener === 'object' &&
+        !!maybeServerListener.serverWillStop,
     );
+    this.toDispose.add(async () => {
+      await Promise.all(
+        serverListeners.map(({ serverWillStop }) => serverWillStop?.()),
+      );
+    });
   }
 
   public async stop() {
-    this.toDispose.forEach(dispose => dispose());
+    await Promise.all([...this.toDispose].map(dispose => dispose()));
     if (this.subscriptionServer) await this.subscriptionServer.close();
     if (this.engineReportingAgent) {
       this.engineReportingAgent.stop();
-      await this.engineReportingAgent.sendAllReports();
+      await this.engineReportingAgent.sendAllReportsAndReportErrors();
     }
   }
 

packages/apollo-server-core/src/types.ts

@@ -124,6 +124,7 @@ export interface Config extends BaseConfig {
   playground?: PlaygroundConfig;
   gateway?: GraphQLService;
   experimental_approximateDocumentStoreMiB?: number;
+  handleSignals?: boolean;
 }
 
 export interface FileUploadOptions {

packages/apollo-server-core/src/utils/pluginTestHarness.ts

@@ -17,6 +17,7 @@ import {
 import {
   ApolloServerPlugin,
   GraphQLRequestExecutionListener,
+  GraphQLServerListener,
 } from 'apollo-server-plugin-base';
 import { InMemoryLRUCache } from 'apollo-server-caching';
 import { Dispatcher } from './dispatcher';
@@ -98,16 +99,19 @@ export default async function pluginTestHarness<TContext>({
   }
 
   const schemaHash = generateSchemaHash(schema);
+  let serverListener: GraphQLServerListener | undefined;
   if (typeof pluginInstance.serverWillStart === 'function') {
-    pluginInstance.serverWillStart({
+    const maybeListener = await pluginInstance.serverWillStart({
       logger: logger || console,
       schema,
       schemaHash,
       engine: {},
     });
+    if (maybeListener && maybeListener.serverWillStop) {
+      serverListener = maybeListener;
+    }
   }
 
-
   const requestContext: GraphQLRequestContext<TContext> = {
     logger: logger || console,
     schema,
@@ -188,5 +192,7 @@ export default async function pluginTestHarness<TContext>({
     requestContext as GraphQLRequestContextWillSendResponse<TContext>,
   );
 
+  await serverListener?.serverWillStop?.();
+
   return requestContext as GraphQLRequestContextWillSendResponse<TContext>;
 }

packages/apollo-server-plugin-base/src/index.ts

@@ -55,13 +55,19 @@ export {
 
 export interface ApolloServerPlugin<
   TContext extends BaseContext = BaseContext
-> {
-  serverWillStart?(service: GraphQLServiceContext): ValueOrPromise<void>;
+> extends AnyFunctionMap {
+  serverWillStart?(
+    service: GraphQLServiceContext,
+  ): ValueOrPromise<GraphQLServerListener | void>;
   requestDidStart?(
     requestContext: GraphQLRequestContext<TContext>,
   ): GraphQLRequestListener<TContext> | void;
 }
 
+export interface GraphQLServerListener {
+  serverWillStop?(): ValueOrPromise<void>;
+}
+
 export type GraphQLRequestListenerParsingDidEnd = (err?: Error) => void;
 export type GraphQLRequestListenerValidationDidEnd =
   ((err?: ReadonlyArray<Error>) => void);

packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts

@@ -5,6 +5,7 @@ import {
   GraphQLServiceContext,
   GraphQLRequestListener,
   GraphQLRequestContext,
+  GraphQLServerListener,
 } from 'apollo-server-plugin-base';
 import {
   /**
@@ -111,7 +112,7 @@ for observability purposes, but all operations will be permitted.`,
       schema,
       schemaHash,
       engine,
-    }: GraphQLServiceContext): Promise<void> {
+    }: GraphQLServiceContext): Promise<GraphQLServerListener> {
       logger.debug('Initializing operation registry plugin.');
 
       assert.ok(schema instanceof GraphQLSchema);
@@ -142,6 +143,12 @@ for observability purposes, but all operations will be permitted.`,
       });
 
       await agent.start();
+
+      return {
+        serverWillStop() {
+          agent.stop();
+        },
+      };
     },
 
     requestDidStart(): GraphQLRequestListener<any> {