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 and renamed to `stopOnTerminationSignals` as a new ApolloServer
option. The new implementation also skips installing the signals handlers by
default if NODE_ENV=test or if you don't appear to be running in Node (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.

(This commit adds new documentation for `stopOnTerminationSignals` to the API
reference but does not change the documentation of
`EngineReportingOptions.handleSignals` on that page because a later commit in
this PR removes that section entirely.)

Author: glasser

docs/source/api/apollo-server.md

@@ -289,6 +289,27 @@ Provide this function to transform the structure of GraphQL response objects bef
 <tr>
 <td colspan="2">
 
+**Lifecycle options**
+</td>
+</tr>
+
+<tr>
+<td>
+
+###### `stopOnTerminationSignals`
+
+`Boolean`
+</td>
+<td>
+
+By default (when running in Node and when the `NODE_ENV` environment variable does not equal `test`), 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 so that process shutdown can continue. Set this to false to disable this behavior, or to true to enable this behavior even when `NODE_ENV` is `test`. 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 `process.on('exit')` handler.
+
+</td>
+</tr>
+
+<tr>
+<td colspan="2">
+
 **Debugging options**
 </td>
 </tr>

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({stopOnTerminationSignals: 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

@@ -31,6 +31,7 @@ import {
 import {
   ApolloServerPlugin,
   GraphQLServiceContext,
+  GraphQLServerListener,
 } from 'apollo-server-plugin-base';
 import runtimeSupportsUploads from './utils/runtimeSupportsUploads';
 
@@ -76,13 +77,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 isNodeLike from './utils/isNodeLike';
 
 const NoIntrospection = (context: ValidationContext) => ({
   Field(node: FieldDefinitionNode) {
@@ -149,7 +151,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'];
 
@@ -177,6 +179,7 @@ export class ApolloServerBase {
       gateway,
       cacheControl,
       experimental_approximateDocumentStoreMiB,
+      stopOnTerminationSignals,
       ...requestOptions
     } = config;
 
@@ -385,6 +388,32 @@ export class ApolloServerBase {
     // is populated accordingly.
     this.ensurePluginInstantiation(plugins);
 
+    // We handle signals if it was explicitly requested, or if we're in Node,
+    // not in a test, and it wasn't explicitly turned off. (For backwards
+    // compatibility, we check both 'stopOnTerminationSignals' and
+    // 'engine.handleSignals'.)
+    if (
+      typeof stopOnTerminationSignals === 'boolean'
+        ? stopOnTerminationSignals
+        : typeof this.config.engine === 'object' &&
+          typeof this.config.engine.handleSignals === 'boolean'
+        ? this.config.engine.handleSignals
+        : isNodeLike && process.env.NODE_ENV !== 'test'
+    ) {
+      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
@@ -585,24 +614,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;
+  stopOnTerminationSignals?: 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 maybeServerListener = await pluginInstance.serverWillStart({
       logger: logger || console,
       schema,
       schemaHash,
       engine: {},
     });
+    if (maybeServerListener && maybeServerListener.serverWillStop) {
+      serverListener = maybeServerListener;
+    }
   }
 
-
   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({stopOnTerminationSignals: 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({ stopOnTerminationSignals: 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,
+          stopOnTerminationSignals: false,
         });
         app = new Server({ port });
 
@@ -514,6 +515,7 @@ const port = 0;
           server = new ApolloServer({
             typeDefs,
             resolvers,
+            stopOnTerminationSignals: false,
             context: () => {
               throw new AuthenticationError('valid result');
             },
@@ -562,6 +564,7 @@ const port = 0;
                 },
               },
             },
+            stopOnTerminationSignals: false,
           });
 
           app = new Server({ port });
@@ -609,6 +612,7 @@ const port = 0;
                 },
               },
             },
+            stopOnTerminationSignals: false,
           });
 
           app = new Server({ port });
@@ -653,6 +657,7 @@ const port = 0;
                 },
               },
             },
+            stopOnTerminationSignals: false,
           });
 
           app = new Server({ port });