Make ApolloServer.stop invoke ApolloGateway.stop (#4907)

Because AS is what invokes ApolloGateway.load, it should be its responsibility
to invoke the matching stop method.

apollographql/federation#452 (aimed at @apollo/gateway
0.23) will change ApolloGateway to no longer "unref" its polling timer: ie, it
makes calling `stop()` actually part of its expected API instead of something
you can ignore if you feel like it without affecting program shutdown. It has
a bit of a hack to still unref the timer if it looks like you're using an
old (pre-2.18) version of Apollo Server, but this PR (which will be released in
v2.20.0) will make ApolloServer stop the gateway for you.

Part of fixing #4428.

Also simplify typings of toDispose set. 
`() => ValueOrPromise<void>` is a weird type because `void` means "I'm not going
to look at the return value" which is sorta incompatible with "but I need to see
if it's a Promise or not". So just make these all async (changing a couple
implementations by adding `async`).

CHANGELOG.md

@@ -11,6 +11,7 @@ The version headers in this history reflect the versions of Apollo Server itself
 
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  With few exceptions, the format of the entry should follow convention (i.e., prefix with package name, use markdown `backtick formatting` for package names and code, suffix with a link to the change-set à la `[PR #YYY](https://link/pull/YYY)`, etc.).  When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.
 
+- `apollo-server-core`: When used with `ApolloGateway`, `ApolloServer.stop` now invokes `ApolloGateway.stop`. (This makes sense because `ApolloServer` already invokes `ApolloGateway.load` which is what starts the behavior stopped by `ApolloGateway.stop`.) Note that `@apollo/gateway` 0.23 will expect to be stopped in order for natural program shutdown to occur. [Issue #4428](https://github.com/apollographql/apollo-server/issues/4428)
 - `apollo-server-core`: Avoid instrumenting schemas for the old `graphql-extensions` library unless extensions are provided. [PR #4893](https://github.com/apollographql/apollo-server/pull/4893) [Issue #4889](https://github.com/apollographql/apollo-server/issues/4889)
 - `apollo-server-plugin-response-cache`: The `shouldReadFromCache` and `shouldWriteToCache` hooks were always documented as returning `ValueOrPromise<boolean>` (ie, that they could be either sync or async), but they actually only worked if they returned a bool. Now they can be either sync or async as intended. [PR #4890](http://github.com/apollographql/apollo-server/pull/4890) [Issue #4886](https://github.com/apollographql/apollo-server/issues/4886)
 

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

@@ -76,7 +76,7 @@ import {
 import { Headers } from 'apollo-server-env';
 import { buildServiceDefinition } from '@apollographql/apollo-tools';
 import { plugin as pluginTracing } from "apollo-tracing";
-import { Logger, SchemaHash, ValueOrPromise, ApolloConfig } from "apollo-server-types";
+import { Logger, SchemaHash, ApolloConfig } from "apollo-server-types";
 import {
   plugin as pluginCacheControl,
   CacheControlExtensionOptions,
@@ -148,7 +148,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<() => ValueOrPromise<void>>();
+  private toDispose = new Set<() => Promise<void>>();
   private experimental_approximateDocumentStoreMiB:
     Config['experimental_approximateDocumentStoreMiB'];
 
@@ -364,7 +364,7 @@ export class ApolloServerBase {
           process.kill(process.pid, signal);
         };
         process.once(signal, handler);
-        this.toDispose.add(() => {
+        this.toDispose.add(async () => {
           process.removeListener(signal, handler);
         });
       });
@@ -388,16 +388,15 @@ export class ApolloServerBase {
       parseOptions,
     } = this.config;
     if (gateway) {
-      this.toDispose.add(
-        // Store the unsubscribe handles, which are returned from
-        // `onSchemaChange`, for later disposal when the server stops
-        gateway.onSchemaChange(
-          schema =>
-            (this.schemaDerivedData = Promise.resolve(
-              this.generateSchemaDerivedData(schema),
-            )),
-        ),
+      // Store the unsubscribe handles, which are returned from
+      // `onSchemaChange`, for later disposal when the server stops
+      const unsubscriber = gateway.onSchemaChange(
+        (schema) =>
+          (this.schemaDerivedData = Promise.resolve(
+            this.generateSchemaDerivedData(schema),
+          )),
       );
+      this.toDispose.add(async () => unsubscriber());
 
       // For backwards compatibility with old versions of @apollo/gateway.
       const engineConfig =
@@ -415,17 +414,22 @@ export class ApolloServerBase {
       // a federated schema!
       this.requestOptions.executor = gateway.executor;
 
-      return gateway.load({ apollo: this.apolloConfig, engine: engineConfig })
-        .then(config => config.schema)
-        .catch(err => {
+      return gateway
+        .load({ apollo: this.apolloConfig, engine: engineConfig })
+        .then((config) => {
+          this.toDispose.add(async () => await gateway.stop?.());
+          return config.schema;
+        })
+        .catch((err) => {
           // We intentionally do not re-throw the exact error from the gateway
           // configuration as it may contain implementation details and this
           // error will propagate to the client. We will, however, log the error
           // for observation in the logs.
-          const message = "This data graph is missing a valid configuration.";
-          this.logger.error(message + " " + (err && err.message || err));
+          const message = 'This data graph is missing a valid configuration.';
+          this.logger.error(message + ' ' + ((err && err.message) || err));
           throw new Error(
-            message + " More details may be available in the server logs.");
+            message + ' More details may be available in the server logs.',
+          );
         });
     }
 
@@ -593,7 +597,7 @@ export class ApolloServerBase {
 
   public async stop() {
     await Promise.all([...this.toDispose].map(dispose => dispose()));
-    if (this.subscriptionServer) await this.subscriptionServer.close();
+    if (this.subscriptionServer) this.subscriptionServer.close();
   }
 
   public installSubscriptionHandlers(server: HttpServer | HttpsServer | Http2Server | Http2SecureServer | WebSocket.Server) {

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

@@ -99,6 +99,7 @@ export interface GraphQLService {
   executor<TContext>(
     requestContext: GraphQLRequestContextExecutionDidStart<TContext>,
   ): ValueOrPromise<GraphQLExecutionResult>;
+  stop?(): Promise<void>;
 }
 
 // This configuration is shared between all integrations and should include