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.

Author: glasser

docs/source/api/apollo-server.md

@@ -275,6 +275,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>
@@ -658,7 +679,7 @@ These are the supported fields of the `engine` object you provide to the [`Apoll
 | `requestAgent` | `http.Agent` or `https.Agent` or `false` | An HTTP(S) agent to use for metrics reporting. Can be either an [`http.Agent`](https://nodejs.org/docs/latest-v10.x/api/http.html#http_class_http_agent) or an [`https.Agent`](https://nodejs.org/docs/latest-v10.x/api/https.html#https_class_https_agent). It behaves the same as the `agent` parameter to [`http.request`](https://nodejs.org/docs/latest-v8.x/api/http.html#http_http_request_options_callback). |
 | `generateClientInfo` | `Function` | <p>Specify this function to provide Apollo Studio with client details for each processed operation. Apollo Studio uses this information to [segment metrics by client](https://www.apollographql.com/docs/studio/client-awareness/).</p><p>The function is passed a [`GraphQLRequestContext`](https://github.com/apollographql/apollo-server/blob/main/packages/apollo-server-types/src/index.ts#L95-L130) object containing all available information about the request. It should return a [`ClientInfo`](https://github.com/apollographql/apollo-server/blob/main/packages/apollo-engine-reporting/src/agent.ts#L35-L39) object describing the associated GraphQL client.</p><p>By default, Apollo Server attempts to obtain `ClientInfo` fields from the `clientInfo` field of the GraphQL operation's `extensions`.</p><p>For advanced use cases when you already use an opaque string to identify your client (such as an API key, x509 certificate, or team codename), use the `clientReferenceId` field to add a reference to that internal identity. The reference ID is not displayed in Studio, but it is available for cross-correspondence, so names and reference IDs should have a one-to-one relationship.</p><p>**Warning:** If you specify a `clientReferenceId`, Graph Manager will treat the `clientName` as a secondary lookup, so changing a `clientName` may result in an unwanted experience.</p>|
 | `calculateSignature` | `Function` | <p>A custom function to use to calculate the "signature" of the schema that operations are running against. This enables Apollo Studio to detect when two non-identical schema strings represent the exact same underlying model.</p><p>For an example, see the [default signature function](https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-graphql/src/operationId.ts), which sorts types and fields, removes extraneous whitespace, and removes unused definitions.</p>  |
-| `handleSignals` | `Boolean` | <p>Set to `false` to disable the Apollo Server trace reporting agent's default signal handling behavior.</p><p>By default, the agent listens for `SIGINT` and `SIGTERM`. Upon receiving either signal, the agent stops, sends a final report, and sends the signal back to itself.</p><p>In addition to disabling the default behavior, you can manually invoke [`stop` and `sendReport`](https://github.com/apollographql/apollo-server/blob/main/packages/apollo-engine-reporting/src/agent.ts) on other signals. Note that `sendReport` is asynchronous, so it should not be called in an `exit` handler.</p> |
+| `handleSignals` | `Boolean` | <p>For backwards compatibility only; specifying `new ApolloServer({engine: {handleSignals: false}})` is equivalent to specifying `new ApolloServer({stopOnTerminationSignals: false})`</p>|
 
 ##### Valid `sendHeaders` object signatures
 

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 });