draft: async server.start() function

Full description to come, but tl;dr: Previously server startup worked like this: - AS constructor runs - If no gateway, calculate schema and schema derived data immediately - If gateway, kick off gateway.load from the end of the constructor, and if it async-throws, log an error once and make the server kinda broken forever - At various spots in the framework integration code, call (but not await) protected willStart, which is an async function that first waits for the gateway to load the schema if necessary and then runs serverWillStart plugin functions; save the Promise returned by calling this. - At request time in the framework integration code, await that promise. And also if there's no schema fail with an error. Now server startup works like this: - There's an explicit state machine situation inside AS - AS constructor initializes state with schema directly if no gateway - If there is a gateway the constructor DOES NOT KICK OFF gateway.load - You can now call `await server.start()` yourself, which will first await gateway.load if necessary, and then await all serverWillStart calls - If you're using `apollo-server` rather than an integration, `server.listen()` will just transparently do this for you; explicit `start()` is just for integrations - The integration places that used to call willStart now call `server.ensureStarting()` instead which will kick off server.start in the background if you didn't (and log any errors thrown). - The places that used to await promiseWillStart no longer do so; generally right after that code we end up calling graphqlServerOptions - graphqlServerOptions now awaits `server.ensureStarted` which will start the server if necessary and throw if it threw. Overall changes: - If you're using `apollo-server`, startup errors will cause `listen` to reject, nothing else necessary. - If you're using an integration you are encouraged to call `await server.start()` yourself after the constructor, which will let you detect startup errors. - But if you don't do that, the server will still start up with similar properties. gateway.load won't start until the integration's `ensureStarting` or graphqlServerOptions' `ensuresStarted` though. Errors will be logged. - Also if you don't call `server.start()`, the full stack trace of any startup error will be logged on *every* failed graphql request instead of just a short message suggesting there's more in logs (but it's only at the beginning of your logs) Yes this is the tl;dr version that I jotted off at the end of my work day off the top of my head without going back and skimming through the PR for details :) Fixes #4921. Fixes apollographql/federation#335.
apollographql · Mar 4, 2021 · 54b9cbd · 54b9cbd
1 parent 1925335
commit 54b9cbd
Show file tree

Hide file tree

Showing 46 changed files with 784 additions and 498 deletions.
diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md
@@ -681,6 +681,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME add start here
 
 const app = express();
 

diff --git a/docs/source/data/subscriptions.mdx b/docs/source/data/subscriptions.mdx
@@ -479,7 +479,7 @@ const express = require('express');
 const PORT = 4000;
 const app = express();
 const server = new ApolloServer({ typeDefs, resolvers });
-
+// FIXME start
 server.applyMiddleware({app})
 
 const httpServer = http.createServer(app);

diff --git a/docs/source/deployment/azure-functions.md b/docs/source/deployment/azure-functions.md
@@ -64,7 +64,7 @@ npm init -y
 npm install apollo-server-azure-functions graphql
 ```
 
-Copy the code below and paste at you **index.js** file.
+Copy the code below and paste in your **index.js** file.
 
 ```javascript
 const { ApolloServer, gql } = require('apollo-server-azure-functions');
@@ -84,7 +84,7 @@ const resolvers = {
 };
 
 const server = new ApolloServer({ typeDefs, resolvers });
-
+// FIXME start
 exports.graphqlHandler = server.createHandler();
 ```
 

diff --git a/docs/source/deployment/lambda.md b/docs/source/deployment/lambda.md
@@ -52,6 +52,7 @@ const resolvers = {
 };
 
 const server = new ApolloServer({ typeDefs, resolvers });
+// FIXME start
 
 exports.graphqlHandler = server.createHandler();
 ```
@@ -164,6 +165,7 @@ const server = new ApolloServer({
     context,
   }),
 });
+// FIXME start
 
 exports.graphqlHandler = server.createHandler();
 ```
@@ -190,6 +192,7 @@ const resolvers = {
 };
 
 const server = new ApolloServer({ typeDefs, resolvers });
+// FIXME start
 
 exports.graphqlHandler = server.createHandler({
   cors: {
@@ -219,6 +222,7 @@ const resolvers = {
 };
 
 const server = new ApolloServer({ typeDefs, resolvers });
+// FIXME start
 
 exports.graphqlHandler = server.createHandler({
   cors: {

diff --git a/docs/source/deployment/netlify.md b/docs/source/deployment/netlify.md
@@ -84,13 +84,14 @@ const server = new ApolloServer({
   typeDefs,
   resolvers
 });
+// FIXME start
 
 exports.handler = server.createHandler();
 ```
 
 Now, make sure you've run `NODE_ENV=development npm run start:lambda`, and navigate to `localhost:9000/graphql` in your browser. You should see GraphQL Playground, where you can run queries against your API!
 
-*Note - The GraphQL Playground will only run if your `NODE_ENV` is set to `development`. If you don't pass this, or your `NODE_ENV` is set to `production`, you will not see the GraphQL Playground.* 
+*Note - The GraphQL Playground will only run if your `NODE_ENV` is set to `development`. If you don't pass this, or your `NODE_ENV` is set to `production`, you will not see the GraphQL Playground.*
 
 ![Local GraphQL Server](../images/graphql.png)
 

diff --git a/docs/source/integrations/middleware.md b/docs/source/integrations/middleware.md
@@ -41,6 +41,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME start
 
 server.applyMiddleware({ app });
 

diff --git a/docs/source/security/terminating-ssl.md b/docs/source/security/terminating-ssl.md
@@ -30,14 +30,15 @@ const environment = process.env.NODE_ENV || 'production'
 const config = configurations[environment]
 
 const apollo = new ApolloServer({ typeDefs, resolvers })
+// FIXME start
 
 const app = express()
 apollo.applyMiddleware({ app })
 
 // Create the HTTPS or HTTP server, per configuration
 var server
 if (config.ssl) {
-  // Assumes certificates are in a .ssl folder off of the package root. Make sure 
+  // Assumes certificates are in a .ssl folder off of the package root. Make sure
   // these files are secured.
   server = https.createServer(
     {

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -56,6 +56,7 @@
   },
   "devDependencies": {
     "@apollographql/graphql-upload-8-fork": "8.1.3",
+    "@josephg/resolvable": "^1.0.0",
     "@types/async-retry": "1.4.2",
     "@types/aws-lambda": "8.10.66",
     "@types/body-parser": "1.19.0",

diff --git a/packages/apollo-server-azure-functions/README.md b/packages/apollo-server-azure-functions/README.md
@@ -31,6 +31,7 @@ const resolvers = {
 };
 
 const server = new ApolloServer({ typeDefs, resolvers });
+// FIXME start
 
 module.exports = server.createHandler();
 ```
@@ -86,6 +87,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME start
 
 module.exports = server.createHandler({
   cors: {
@@ -118,6 +120,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME start
 
 module.exports = server.createHandler({
   cors: {

diff --git a/packages/apollo-server-azure-functions/src/ApolloServer.ts b/packages/apollo-server-azure-functions/src/ApolloServer.ts
@@ -36,10 +36,10 @@ export class ApolloServer extends ApolloServerBase {
   }
 
   public createHandler({ cors }: CreateHandlerOptions = { cors: undefined }) {
-    // We will kick off the `willStart` event once for the server, and then
-    // await it before processing any requests by incorporating its `await` into
-    // the GraphQLServerOptions function which is called before each request.
-    const promiseWillStart = this.willStart();
+    // In case the user didn't bother to call and await the `start` method, we
+    // kick it off in the background (with any errors getting logged
+    // and also rethrown from graphQLServerOptions during later requests).
+    this.ensureStarting();
 
     const corsHeaders: HttpResponse['headers'] = {};
 
@@ -145,7 +145,6 @@ export class ApolloServer extends ApolloServerBase {
         );
       };
       graphqlAzureFunction(async () => {
-        await promiseWillStart;
         return this.createGraphQLServerOptions(req, context);
       })(context, req, callbackFilter);
     };

diff --git a/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts b/packages/apollo-server-cache-memcached/src/__tests__/Memcached.test.ts
@@ -9,6 +9,9 @@ import {
 
 describe('Memcached', () => {
   const cache = new MemcachedCache('localhost');
+  afterAll(async () => {
+    await cache.close();
+  })
   testKeyValueCache_Basics(cache);
   testKeyValueCache_Expiration(cache);
 });
diff --git a/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts b/packages/apollo-server-cache-redis/src/__mocks__/ioredis.ts
@@ -1,38 +1,53 @@
-const IORedis = jest.genMockFromModule('ioredis');
+class Redis {
+  private keyValue = {};
+  private timeouts = new Set<NodeJS.Timer>();
 
-const keyValue = {};
+  async del(key: string) {
+    delete this.keyValue[key];
+    return true;
+  }
 
-const deleteKey = key => {
-  delete keyValue[key];
-  return Promise.resolve(true);
-};
+  async get(key: string) {
+    if (this.keyValue[key]) {
+      return this.keyValue[key].value;
+    }
+  }
 
-const getKey = key => {
-  if (keyValue[key]) {
-    return Promise.resolve(keyValue[key].value);
+  async mget(...keys: string[]) {
+    return keys.map((key) => {
+      if (this.keyValue[key]) {
+        return this.keyValue[key].value;
+      }
+    });
   }
 
-  return Promise.resolve(undefined);
-};
+  async set(key, value, type, ttl) {
+    this.keyValue[key] = {
+      value,
+      ttl,
+    };
+    if (ttl) {
+      const timeout = setTimeout(() => {
+        this.timeouts.delete(timeout);
+        delete this.keyValue[key];
+      }, ttl * 1000);
+      this.timeouts.add(timeout);
+    }
+    return true;
+  }
+
+  nodes() {
+    return [];
+  }
 
-const mGetKey = (key, cb) => getKey(key).then(val => [val]);
+  async flushdb() {}
 
-const setKey = (key, value, type, ttl) => {
-  keyValue[key] = {
-    value,
-    ttl,
-  };
-  if (ttl) {
-    setTimeout(() => {
-      delete keyValue[key];
-    }, ttl * 1000);
+  async quit() {
+    this.timeouts.forEach((t) => clearTimeout(t));
   }
-  return Promise.resolve(true);
-};
+}
 
-IORedis.prototype.del.mockImplementation(deleteKey);
-IORedis.prototype.get.mockImplementation(getKey);
-IORedis.prototype.mget.mockImplementation(mGetKey);
-IORedis.prototype.set.mockImplementation(setKey);
+// Use the same mock as Redis.Cluster.
+(Redis as any).Cluster = Redis;
 
-export default IORedis;
+export default Redis;
diff --git a/packages/apollo-server-cache-redis/src/__tests__/RedisCache.test.ts b/packages/apollo-server-cache-redis/src/__tests__/RedisCache.test.ts
@@ -7,7 +7,10 @@ import {
 } from '../../../apollo-server-caching/src/__tests__/testsuite';
 
 describe('Redis', () => {
-  const cache = new RedisCache({ host: 'localhost' });
+  const cache = new RedisCache();
+  afterAll(async () => {
+    await cache.close();
+  })
   testKeyValueCache_Basics(cache);
   testKeyValueCache_Expiration(cache);
 });
diff --git a/packages/apollo-server-caching/src/__tests__/testsuite.ts b/packages/apollo-server-caching/src/__tests__/testsuite.ts
@@ -36,8 +36,8 @@ export function testKeyValueCache_Expiration(
     });
 
     afterAll(() => {
+      jest.useRealTimers();
       unmockDate();
-      keyValueCache.close && keyValueCache.close();
     });
 
     it('is able to expire keys based on ttl', async () => {
@@ -70,6 +70,11 @@ export function testKeyValueCache_Expiration(
 
 export function testKeyValueCache(keyValueCache: TestableKeyValueCache) {
   describe('KeyValueCache Test Suite', () => {
+    afterAll(async () => {
+      if (keyValueCache.close) {
+      await keyValueCache.close();
+      }
+    })
     testKeyValueCache_Basics(keyValueCache);
     testKeyValueCache_Expiration(keyValueCache);
   });

diff --git a/packages/apollo-server-cloud-functions/README.md b/packages/apollo-server-cloud-functions/README.md
@@ -37,6 +37,7 @@ const server = new ApolloServer({
   playground: true,
   introspection: true,
 });
+// FIXME start
 
 exports.handler = server.createHandler();
 ```
@@ -82,6 +83,7 @@ const server = new ApolloServer({
     res,
   }),
 });
+// FIXME start
 
 exports.handler = server.createHandler();
 ```
@@ -111,6 +113,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME start
 
 exports.handler = server.createHandler({
   cors: {
@@ -143,6 +146,7 @@ const server = new ApolloServer({
   typeDefs,
   resolvers,
 });
+// FIXME start
 
 exports.handler = server.createHandler({
   cors: {

diff --git a/packages/apollo-server-cloud-functions/src/ApolloServer.ts b/packages/apollo-server-cloud-functions/src/ApolloServer.ts
@@ -34,10 +34,10 @@ export class ApolloServer extends ApolloServerBase {
   }
 
   public createHandler({ cors }: CreateHandlerOptions = { cors: undefined }) {
-    // We will kick off the `willStart` event once for the server, and then
-    // await it before processing any requests by incorporating its `await` into
-    // the GraphQLServerOptions function which is called before each request.
-    const promiseWillStart = this.willStart();
+    // In case the user didn't bother to call and await the `start` method, we
+    // kick it off in the background (with any errors getting logged
+    // and also rethrown from graphQLServerOptions during later requests).
+    this.ensureStarting();
 
     const corsHeaders = {} as Record<string, any>;
 
@@ -129,14 +129,6 @@ export class ApolloServer extends ApolloServerBase {
       }
 
       graphqlCloudFunction(async () => {
-        // In a world where this `createHandler` was async, we might avoid this
-        // but since we don't want to introduce a breaking change to this API
-        // (by switching it to `async`), we'll leverage the
-        // `GraphQLServerOptions`, which are dynamically built on each request,
-        // to `await` the `promiseWillStart` which we kicked off at the top of
-        // this method to ensure that it runs to completion (which is part of
-        // its contract) prior to processing the request.
-        await promiseWillStart;
         return this.createGraphQLServerOptions(req, res);
       })(req, res);
     };

diff --git a/packages/apollo-server-cloudflare/src/ApolloServer.ts b/packages/apollo-server-cloudflare/src/ApolloServer.ts
@@ -14,7 +14,11 @@ export class ApolloServer extends ApolloServerBase {
   }
 
   public async listen() {
-    await this.willStart();
+    // In case the user didn't bother to call and await the `start` method, we
+    // kick it off in the background (with any errors getting logged
+    // and also rethrown from graphQLServerOptions during later requests).
+    this.ensureStarting();
+
     addEventListener('fetch', (event: FetchEvent) => {
       event.respondWith(
         graphqlCloudflare(() => {

diff --git a/packages/apollo-server-core/package.json b/packages/apollo-server-core/package.json
@@ -28,6 +28,7 @@
     "@apollographql/apollo-tools": "^0.4.3",
     "@apollographql/graphql-playground-html": "1.6.27",
     "@apollographql/graphql-upload-8-fork": "^8.1.3",
+    "@josephg/resolvable": "^1.0.0",
     "@types/ws": "^7.0.0",
     "apollo-cache-control": "file:../apollo-cache-control",
     "apollo-datasource": "file:../apollo-datasource",