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 new
implementation also skips installing the signals handlers by default if
NODE_ENV=test (and we update some tests that explicitly set other NODE_ENVs to
set handleSignals: false)

The main effect on existing code is that on one of these signals, any
SubscriptionServer and ApolloGateway will be stopped in addition to any
EngineReportingAgent.

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,7 +73,7 @@ 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,
@@ -145,7 +146,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 +174,7 @@ export class ApolloServerBase {
       gateway,
       cacheControl,
       experimental_approximateDocumentStoreMiB,
+      handleSignals,
       ...requestOptions
     } = config;
 
@@ -381,6 +383,32 @@ export class ApolloServerBase {
     // is populated accordingly.
     this.ensurePluginInstantiation(plugins);
 
+    // We handle signals if it was explicitly requested, or if we're not in a test
+    // and it wasn't explicitly turned off. (For backwards compatibility, we check
+    // both the top-level handleSignals and nested inside 'engine'.)
+    if (
+      handleSignals === true ||
+      (typeof this.config.engine === 'object' &&
+        this.config.engine.handleSignals === true) ||
+      (process.env.NODE_ENV !== 'test' &&
+        handleSignals !== false &&
+        (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 +609,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-express/src/__tests__/ApolloServer.test.ts

@@ -62,7 +62,7 @@ describe('apollo-server-express', () => {
     serverOptions: ApolloServerExpressConfig,
     options: Partial<ServerRegistration> = {},
   ) {
-    server = new ApolloServer(serverOptions);
+    server = new ApolloServer({handleSignals: false, ...serverOptions});
     app = express();
 
     server.applyMiddleware({ ...options, app });
@@ -184,13 +184,12 @@ describe('apollo-server-express', () => {
     });
 
     it('renders GraphQL playground using request original url', async () => {
-      const nodeEnv = process.env.NODE_ENV;
-      delete process.env.NODE_ENV;
       const samplePath = '/innerSamplePath';
 
       const rewiredServer = new ApolloServer({
         typeDefs,
         resolvers,
+        playground: true,
       });
       const innerApp = express();
       rewiredServer.applyMiddleware({ app: innerApp });
@@ -218,7 +217,6 @@ describe('apollo-server-express', () => {
             },
           },
           (error, response, body) => {
-            process.env.NODE_ENV = nodeEnv;
             if (error) {
               reject(error);
             } else {

packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts

@@ -64,7 +64,7 @@ describe('apollo-server-fastify', () => {
     options: Partial<ServerRegistration> = {},
     mockDecorators: boolean = false,
   ) {
-    server = new ApolloServer(serverOptions);
+    server = new ApolloServer({ handleSignals: false, ...serverOptions });
     app = fastify();
 
     if (mockDecorators) {

packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts

@@ -155,6 +155,7 @@ const port = 0;
         server = new ApolloServer({
           typeDefs,
           resolvers,
+          handleSignals: false,
         });
         app = new Server({ port });
 
@@ -514,6 +515,7 @@ const port = 0;
           server = new ApolloServer({
             typeDefs,
             resolvers,
+            handleSignals: false,
             context: () => {
               throw new AuthenticationError('valid result');
             },
@@ -562,6 +564,7 @@ const port = 0;
                 },
               },
             },
+            handleSignals: false,
           });
 
           app = new Server({ port });
@@ -609,6 +612,7 @@ const port = 0;
                 },
               },
             },
+            handleSignals: false,
           });
 
           app = new Server({ port });
@@ -653,6 +657,7 @@ const port = 0;
                 },
               },
             },
+            handleSignals: false,
           });
 
           app = new Server({ port });