diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 5ff97ee9885..021df8bf52d 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -701,6 +701,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 index c7d5878872a..479a0fd354f 100644 --- a/docs/source/data/subscriptions.mdx +++ b/docs/source/data/subscriptions.mdx @@ -422,7 +422,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 index 2c54a1e5294..2a791105d46 100644 --- 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 index 96f73fe376e..599028e9d41 100644 --- 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 index acc30166085..6dd8257d8ae 100644 --- 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 index 474fc098db6..d6c64dc9606 100644 --- 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 index 67925171932..a0f275d83d9 100644 --- a/docs/source/security/terminating-ssl.md +++ b/docs/source/security/terminating-ssl.md @@ -30,6 +30,7 @@ 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 }) @@ -37,7 +38,7 @@ 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 index ef02b7b1c0a..cedf5981f9f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3135,6 +3135,11 @@ } } }, + "@josephg/resolvable": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@josephg/resolvable/-/resolvable-1.0.0.tgz", + "integrity": "sha512-OfTtjoqB2doov5aTJxkyAMK8dXoo7CjCUQSYUEtiY34jbWduOGV7+168tmCT8COMsUEd5DMSFg/0iAOPCBTNAQ==" + }, "@koa/cors": { "version": "2.2.3", "resolved": "https://registry.npmjs.org/@koa/cors/-/cors-2.2.3.tgz", @@ -5587,6 +5592,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:packages/apollo-cache-control", "apollo-datasource": "file:packages/apollo-datasource", diff --git a/package.json b/package.json index 565cc14db4b..6df41cb2123 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,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 index 47906b08920..bba996c400d 100644 --- 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 index 14d663fce7b..984737479d9 100644 --- a/packages/apollo-server-azure-functions/src/ApolloServer.ts +++ b/packages/apollo-server-azure-functions/src/ApolloServer.ts @@ -36,11 +36,6 @@ 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(); - const corsHeaders: HttpResponse['headers'] = {}; if (cors) { @@ -144,7 +139,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 index d3d7d20b47e..e706b0207e5 100644 --- 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 index 61a38a55f19..dd56966b0de 100644 --- 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(); -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 index 9c0741d2744..f9afb7bac3d 100644 --- 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 index 722b6e36923..81841f54601 100644 --- 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 index 7178b1112d4..8fa14d1f902 100644 --- 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 index b5b665f77de..c42f4a61211 100644 --- a/packages/apollo-server-cloud-functions/src/ApolloServer.ts +++ b/packages/apollo-server-cloud-functions/src/ApolloServer.ts @@ -34,11 +34,6 @@ 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(); - const corsHeaders = {} as Record; if (cors) { @@ -129,14 +124,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 index 052b234268e..bf8b1db36cb 100644 --- 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 index 6a883179fd8..3253ec3a554 100644 --- 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", diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index e47801eddec..3fe1679f55e 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -3,8 +3,8 @@ import { addMockFunctionsToSchema, GraphQLParseOptions, } from 'graphql-tools'; -import { Server as NetServer } from 'net' -import { Server as TlsServer } from 'tls' +import { Server as NetServer } from 'net'; +import { Server as TlsServer } from 'tls'; import { Server as HttpServer } from 'http'; import { Http2Server, Http2SecureServer } from 'http2'; import { Server as HttpsServer } from 'https'; @@ -19,10 +19,8 @@ import { ValidationContext, FieldDefinitionNode, DocumentNode, - isObjectType, - isScalarType, - isSchema, } from 'graphql'; +import resolvable, { Resolvable } from '@josephg/resolvable'; import { GraphQLExtension } from 'graphql-extensions'; import { InMemoryLRUCache, @@ -43,10 +41,7 @@ import { import WebSocket from 'ws'; import { formatApolloErrors } from 'apollo-server-errors'; -import { - GraphQLServerOptions, - PersistedQueryOptions, -} from './graphqlOptions'; +import { GraphQLServerOptions, PersistedQueryOptions } from './graphqlOptions'; import { Config, @@ -55,6 +50,7 @@ import { SubscriptionServerOptions, FileUploadOptions, PluginDefinition, + GraphQLService, } from './types'; import { gql } from './index'; @@ -75,13 +71,13 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; -import { plugin as pluginTracing } from "apollo-tracing"; -import { Logger, SchemaHash, ApolloConfig } from "apollo-server-types"; +import { plugin as pluginTracing } from 'apollo-tracing'; +import { Logger, SchemaHash, ApolloConfig } from 'apollo-server-types'; import { plugin as pluginCacheControl, CacheControlExtensionOptions, } from 'apollo-cache-control'; -import { cloneObject } from "./runHttpQuery"; +import { cloneObject } from './runHttpQuery'; import isNodeLike from './utils/isNodeLike'; import { determineApolloConfig } from './determineApolloConfig'; import { @@ -115,20 +111,47 @@ function approximateObjectSize(obj: T): number { } type SchemaDerivedData = { + schema: GraphQLSchema; + schemaHash: SchemaHash; + extensions: Array<() => GraphQLExtension>; // A store that, when enabled (default), will store the parsed and validated // versions of operations in-memory, allowing subsequent parses/validates // on the same operation to be executed immediately. documentStore?: InMemoryLRUCache; - schema: GraphQLSchema; - schemaHash: SchemaHash; - extensions: Array<() => GraphQLExtension>; }; +type ServerState = + | { phase: 'initialized with schema'; schemaDerivedData: SchemaDerivedData } + | { phase: 'initialized with gateway'; gateway: GraphQLService } + | { phase: 'starting'; barrier: Resolvable } + | { + phase: 'invoking serverWillStart'; + barrier: Resolvable; + schemaDerivedData: SchemaDerivedData; + } + | { phase: 'failed to start'; error: Error } + | { + phase: 'started'; + schemaDerivedData: SchemaDerivedData; + } + | { phase: 'stopping'; barrier: Resolvable } + | { phase: 'stopped'; stopError: Error | null }; + +// Throw this in places that should be unreachable (because all other cases have +// been handled, reducing the type of the argument to `never`). TypeScript will +// complain if in fact there is a valid type for the argument. +class UnreachableCaseError extends Error { + constructor(val: never) { + super(`Unreachable case: ${val}`); + } +} export class ApolloServerBase { private logger: Logger; public subscriptionsPath?: string; public graphqlPath: string = '/graphql'; - public requestOptions: Partial> = Object.create(null); + public requestOptions: Partial> = Object.create( + null, + ); private context?: Context | ContextFunction; private apolloConfig: ApolloConfig; @@ -144,14 +167,13 @@ export class ApolloServerBase { protected playgroundOptions?: PlaygroundRenderPageOptions; private parseOptions: GraphQLParseOptions; - private schemaDerivedData: Promise; private config: Config; + private state: ServerState; /** @deprecated: This is undefined for servers operating as gateways, and will be removed in a future release **/ protected schema?: GraphQLSchema; private toDispose = new Set<() => Promise>(); private toDisposeLast = new Set<() => Promise>(); - private experimental_approximateDocumentStoreMiB: - Config['experimental_approximateDocumentStoreMiB']; + private experimental_approximateDocumentStoreMiB: Config['experimental_approximateDocumentStoreMiB']; // The constructor should be universal across all environments. All environment specific behavior should be set by adding or overriding methods constructor(config: Config) { @@ -195,7 +217,7 @@ export class ApolloServerBase { this.logger = config.logger; } else { // If the user didn't provide their own logger, we'll initialize one. - const loglevelLogger = loglevel.getLogger("apollo-server"); + const loglevelLogger = loglevel.getLogger('apollo-server'); // We don't do much logging in Apollo Server right now. There's a notion // of a `debug` flag, which changes stack traces in some error messages, @@ -247,10 +269,8 @@ export class ApolloServerBase { } if (requestOptions.persistedQueries !== false) { - const { - cache: apqCache = requestOptions.cache!, - ...apqOtherOptions - } = requestOptions.persistedQueries || Object.create(null); + const { cache: apqCache = requestOptions.cache!, ...apqOtherOptions } = + requestOptions.persistedQueries || Object.create(null); requestOptions.persistedQueries = { cache: new PrefixingKeyValueCache(apqCache, APQ_CACHE_PREFIX), @@ -325,21 +345,6 @@ export class ApolloServerBase { this.playgroundOptions = createPlaygroundOptions(playground); - // TODO: This is a bit nasty because the subscription server needs this.schema synchronously, for reasons of backwards compatibility. - const _schema = this.initSchema(); - - if (isSchema(_schema)) { - const derivedData = this.generateSchemaDerivedData(_schema); - this.schema = derivedData.schema; - this.schemaDerivedData = Promise.resolve(derivedData); - } else if (typeof _schema.then === 'function') { - this.schemaDerivedData = _schema.then(schema => - this.generateSchemaDerivedData(schema), - ); - } else { - throw new Error("Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible."); - } - // Plugins will be instantiated if they aren't already, and this.plugins // is populated accordingly. this.ensurePluginInstantiation(plugins); @@ -389,6 +394,52 @@ export class ApolloServerBase { }); }); } + + if (gateway) { + // ApolloServer has been initialized but we have not yet tried to load the + // schema from the gateway. That will wait until the user calls + // `server.start()`, or until `ensureStarting` or `ensureStarted` are + // called. (In the case of a serverless framework integration, + // `ensureStarting` is automatically called at the end of the + // constructor.) + this.state = { phase: 'initialized with gateway', gateway }; + + // The main thing that the Gateway does is replace execution with + // its own executor. It would be awkward if you always had to pass + // `gateway: gateway, executor: gateway` to this constructor, so + // we let specifying `gateway` be a shorthand for the above. + // (We won't actually invoke the executor until after we're successfully + // called `gateway.load`.) + this.requestOptions.executor = gateway.executor; + } else { + // We construct the schema synchronously so that we can fail fast if + // the schema can't be constructed, and so that installSubscriptionHandlers + // and the deprecated `.schema` field (neither of which have ever worked + // with the gateway) are available immediately after the constructor returns. + this.state = { + phase: 'initialized with schema', + schemaDerivedData: this.generateSchemaDerivedData( + this.constructSchema(), + ), + }; + // This field is deprecated; users who are interested in learning + // their server's schema should instead make a plugin with serverWillStart, + // or register onSchemaChange on their gateway. It is only ever + // set for non-gateway servers. + this.schema = this.state.schemaDerivedData.schema; + } + + // The main entry point (createHandler) to serverless frameworks generally + // needs to be called synchronously from the top level of your entry point, + // unlike (eg) applyMiddleware, so we can't expect you to `await + // server.start()` before calling it. So we kick off the start + // asynchronously from the constructor, and failures are logged and cause + // later requests to fail (in graphQLServerOptions). There's no way to make + // "the whole server fail" separately from making individual requests fail, + // but that's not entirely unreasonable for a "serverless" model. + if (this.serverlessFramework()) { + this.ensureStarting(); + } } // used by integrations to synchronize the path with subscriptions, some @@ -397,9 +448,249 @@ export class ApolloServerBase { this.graphqlPath = path; } - private initSchema(): GraphQLSchema | Promise { + // Awaiting a call to `start` ensures that a schema has been loaded and that + // all plugin `serverWillStart` hooks have been called. If either of these + // processes throw, `start` will (async) throw as well. + // + // If you're using the batteries-included `apollo-server` package, you don't + // need to call `start` yourself (in fact, it will throw if you do so); its + // `listen` method takes care of that for you (this is why the actual logic is + // in the `_start` helper). + // + // If instead you're using an integration package, you are highly encouraged + // to await a call to `start` immediately after creating your `ApolloServer`, + // before attaching it to your web framework and starting to accept requests. + // `start` should only be called once; if it throws and you'd like to retry, + // just create another `ApolloServer`. (Note that this paragraph does not + // apply to "serverless framework" integrations like Lambda.) + // + // For backwards compatibility with the pre-2.22 API, you are not required to + // call start() yourself (this may change in AS3). Most integration packages + // call the protected `ensureStarting` when you first interact with them, + // which kicks off a "background" call to `start` if you haven't called it + // yourself. Then `graphQLServerOptions` (which is called before processing) + // each incoming GraphQL request) calls `ensureStarted` which waits for + // `start` to successfully complete (possibly by calling it itself), and + // throws a redacted error if `start` was not successful. If `start` is + // invoked implicitly by either of these mechanisms, any error that it throws + // will be logged when they occur and then again on every subsequent + // `graphQLServerOptions` call (ie, every GraphQL request). Note that start + // failures are not recoverable without creating a new ApolloServer. You are + // highly encouraged to make these backwards-compatibility paths into no-ops + // by awaiting a call to `start` yourself. + // + // Serverless integrations like Lambda (which override `serverlessFramework()` + // to return true) do not support calling `start()`, because their lifecycle + // doesn't allow you to wait before assigning a handler or allowing the + // handler to be called. So they call `ensureStarting` at the end of the + // constructor, and don't really differentiate between startup failures and + // request failures. This is hopefully appropriate for a "serverless" + // framework. As above, startup failures result in returning a redacted error + // to the end user and logging the more detailed error. + public async start(): Promise { + if (this.serverlessFramework()) { + throw new Error( + 'When using an ApolloServer subclass from a serverless framework ' + + "package, you don't need to call start(); just call createHandler().", + ); + } + + return await this._start(); + } + + protected async _start(): Promise { + const initialState = this.state; + if ( + initialState.phase !== 'initialized with gateway' && + initialState.phase !== 'initialized with schema' + ) { + throw new Error( + `called start() with surprising state ${initialState.phase}`, + ); + } + const barrier = resolvable(); + this.state = { phase: 'starting', barrier }; + try { + const schemaDerivedData = + initialState.phase === 'initialized with schema' + ? initialState.schemaDerivedData + : this.generateSchemaDerivedData( + await this.startGatewayAndLoadSchema(initialState.gateway), + ); + + this.state = { + phase: 'invoking serverWillStart', + barrier, + schemaDerivedData, + }; + + const service: GraphQLServiceContext = { + logger: this.logger, + schema: schemaDerivedData.schema, + schemaHash: schemaDerivedData.schemaHash, + apollo: this.apolloConfig, + serverlessFramework: this.serverlessFramework(), + engine: { + serviceID: this.apolloConfig.graphId, + apiKeyHash: this.apolloConfig.keyHash, + }, + }; + + // The `persistedQueries` attribute on the GraphQLServiceContext was + // originally used by the operation registry, which shared the cache with + // it. This is no longer the case. However, while we are continuing to + // expand the support of the interface for `persistedQueries`, e.g. with + // additions like https://github.com/apollographql/apollo-server/pull/3623, + // we don't want to continually expand the API surface of what we expose + // to the plugin API. In this particular case, it certainly doesn't need + // to get the `ttl` default value which are intended for APQ only. + if (this.requestOptions.persistedQueries?.cache) { + service.persistedQueries = { + cache: this.requestOptions.persistedQueries.cache, + }; + } + + 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?.()), + ); + }); + + this.state = { phase: 'started', schemaDerivedData }; + } catch (error) { + this.state = { phase: 'failed to start', error }; + throw error; + } finally { + barrier.resolve(); + } + } + + // Part of the backwards-compatibility behavior described above `start` + // to make ApolloServer work if you don't explicitly call `start`. This is + // called at the beginning of each GraphQL request by `graphQLServerOptions`. + // It calls `start` for you if it hasn't been called yet, and only returns + // successfully if some call to `start` succeeds. + private async ensureStarted(): Promise { + while (true) { + switch (this.state.phase) { + case 'initialized with gateway': + case 'initialized with schema': + await this.start(); + // continue the while loop + break; + case 'starting': + case 'invoking serverWillStart': + await this.state.barrier; + // continue the while loop + break; + case 'failed to start': + throw this.state.error; + case 'started': + return this.state.schemaDerivedData; + case 'stopping': + throw new Error('Apollo Server is stopping'); + case 'stopped': + throw new Error('Apollo Server has stopped'); + default: + throw new UnreachableCaseError(this.state); + } + } + } + + // Part of the backwards-compatibility behavior described above `start` to + // make ApolloServer work if you don't explicitly call `start`. This is called + // by some of the integration frameworks when you interact with them (eg by + // calling applyMiddleware). It is also called from the end of the constructor + // for serverless framework integrations. + // + // It calls `start` for you if it hasn't been called yet, but doesn't wait for + // `start` to finish. The goal is that if you don't call `start` yourself the + // server should still do the rest of startup vaguely near when your server + // starts, not just when the first GraphQL request comes in. Without this + // call, startup wouldn't occur until `graphQLServerOptions` invokes + // `ensureStarted`. + protected ensureStarting() { + if ( + this.state.phase === 'initialized with gateway' || + this.state.phase === 'initialized with schema' + ) { + // Ah well. It would have been nice if the user had bothered + // to call and await `start()`; that way they'd be able to learn + // about any errors from it. Instead we'll kick it off here. + // Any thrown error will get logged, and also will cause + // every call to graphQLServerOptions (ie, every GraphQL operation) + // to log it again and prevent the operation from running. + this._start().catch((e) => this.logStartupError(e)); + } + } + + // Given an error that occurred during Apollo Server startup, log it with a + // helpful message. Note that this is only used if `ensureStarting` or + // `graphQLServerOptions` had to initiate the startup process; if you call + // `start` yourself (or you're using `apollo-server` whose `listen()` does + // it for you) then you can handle the error however you'd like rather than + // this log occurring. (We don't suggest the use of `start()` for serverless + // frameworks because they don't support it.) + private logStartupError(err: Error) { + const prelude = this.serverlessFramework() + ? 'An error occurred during Apollo Server startup.' + : 'Apollo Server was started implicitly and an error occurred during startup. ' + + '(Consider calling `await server.start()` immediately after ' + + '`server = new ApolloServer()` so you can handle these errors directly before ' + + 'starting your web server.)'; + this.logger.error( + prelude + + ' All GraphQL requests will now fail. The startup error ' + + 'was: ' + + ((err && err.message) || err), + ); + } + + private async startGatewayAndLoadSchema( + gateway: GraphQLService, + ): Promise { + // Store the unsubscribe handles, which are returned from + // `onSchemaChange`, for later disposal when the server stops + const unsubscriber = gateway.onSchemaChange((schema) => { + // If we're still happily running, update our schema-derived state. + if (this.state.phase === 'started') { + this.state.schemaDerivedData = this.generateSchemaDerivedData(schema); + } + }); + this.toDispose.add(async () => unsubscriber()); + + // For backwards compatibility with old versions of @apollo/gateway. + const engineConfig = + this.apolloConfig.keyHash && this.apolloConfig.graphId + ? { + apiKeyHash: this.apolloConfig.keyHash, + graphId: this.apolloConfig.graphId, + graphVariant: this.apolloConfig.graphVariant, + } + : undefined; + + const config = await gateway.load({ + apollo: this.apolloConfig, + engine: engineConfig, + }); + this.toDispose.add(async () => await gateway.stop?.()); + return config.schema; + } + + private constructSchema(): GraphQLSchema { const { - gateway, schema, modules, typeDefs, @@ -407,119 +698,72 @@ export class ApolloServerBase { schemaDirectives, parseOptions, } = this.config; - if (gateway) { - // 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 = - this.apolloConfig.keyHash && this.apolloConfig.graphId - ? { - apiKeyHash: this.apolloConfig.keyHash, - graphId: this.apolloConfig.graphId, - graphVariant: this.apolloConfig.graphVariant, - } - : undefined; - - // Set the executor whether the gateway 'load' call succeeds or not. - // If the schema becomes available eventually (after a setInterval retry) - // this executor will still be necessary in order to be able to support - // a federated schema! - this.requestOptions.executor = gateway.executor; - - 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)); - throw new Error( - message + ' More details may be available in the server logs.', - ); - }); + if (schema) { + return schema; } - let constructedSchema: GraphQLSchema; - if (schema) { - constructedSchema = schema; - } else if (modules) { + if (modules) { const { schema, errors } = buildServiceDefinition(modules); if (errors && errors.length > 0) { - throw new Error(errors.map(error => error.message).join('\n\n')); + throw new Error(errors.map((error) => error.message).join('\n\n')); } - constructedSchema = schema!; - } else { - if (!typeDefs) { - throw Error( - 'Apollo Server requires either an existing schema, modules or typeDefs', - ); - } - - const augmentedTypeDefs = Array.isArray(typeDefs) ? typeDefs : [typeDefs]; + return schema!; + } - // We augment the typeDefs with the @cacheControl directive and associated - // scope enum, so makeExecutableSchema won't fail SDL validation + if (!typeDefs) { + throw Error( + 'Apollo Server requires either an existing schema, modules or typeDefs', + ); + } - if (!isDirectiveDefined(augmentedTypeDefs, 'cacheControl')) { - augmentedTypeDefs.push( - gql` - enum CacheControlScope { - PUBLIC - PRIVATE - } + const augmentedTypeDefs = Array.isArray(typeDefs) ? typeDefs : [typeDefs]; - directive @cacheControl( - maxAge: Int - scope: CacheControlScope - ) on FIELD_DEFINITION | OBJECT | INTERFACE - `, - ); - } + // We augment the typeDefs with the @cacheControl directive and associated + // scope enum, so makeExecutableSchema won't fail SDL validation - if (this.uploadsConfig) { - const { GraphQLUpload } = require('@apollographql/graphql-upload-8-fork'); - if (Array.isArray(resolvers)) { - if (resolvers.every(resolver => !resolver.Upload)) { - resolvers.push({ Upload: GraphQLUpload }); + if (!isDirectiveDefined(augmentedTypeDefs, 'cacheControl')) { + augmentedTypeDefs.push( + gql` + enum CacheControlScope { + PUBLIC + PRIVATE } - } else { - if (resolvers && !resolvers.Upload) { - resolvers.Upload = GraphQLUpload; - } - } - // We augment the typeDefs with the Upload scalar, so typeDefs that - // don't include it won't fail - augmentedTypeDefs.push( - gql` - scalar Upload - `, - ); + directive @cacheControl( + maxAge: Int + scope: CacheControlScope + ) on FIELD_DEFINITION | OBJECT | INTERFACE + `, + ); + } + + if (this.uploadsConfig) { + const { GraphQLUpload } = require('@apollographql/graphql-upload-8-fork'); + if (Array.isArray(resolvers)) { + if (resolvers.every((resolver) => !resolver.Upload)) { + resolvers.push({ Upload: GraphQLUpload }); + } + } else { + if (resolvers && !resolvers.Upload) { + resolvers.Upload = GraphQLUpload; + } } - constructedSchema = makeExecutableSchema({ - typeDefs: augmentedTypeDefs, - schemaDirectives, - resolvers, - parseOptions, - }); + // We augment the typeDefs with the Upload scalar, so typeDefs that + // don't include it won't fail + augmentedTypeDefs.push( + gql` + scalar Upload + `, + ); } - return constructedSchema; + return makeExecutableSchema({ + typeDefs: augmentedTypeDefs, + schemaDirectives, + resolvers, + parseOptions, + }); } private generateSchemaDerivedData(schema: GraphQLSchema): SchemaDerivedData { @@ -556,76 +800,57 @@ export class ApolloServerBase { }; } - protected async willStart() { - try { - var { schema, schemaHash } = await this.schemaDerivedData; - } catch (err) { - // The `schemaDerivedData` can throw if the Promise it points to does not - // resolve with a `GraphQLSchema`. As errors from `willStart` are start-up - // errors, other Apollo middleware after us will not be called, including - // our health check, CORS, etc. - // - // Returning here allows the integration's other Apollo middleware to - // function properly in the event of a failure to obtain the data graph - // configuration from the gateway's `load` method during initialization. + public async stop() { + // Calling stop more than once should have the same result as the first time. + if (this.state.phase === 'stopped') { + if (this.state.stopError) { + throw this.state.stopError; + } return; } - const service: GraphQLServiceContext = { - logger: this.logger, - schema: schema, - schemaHash: schemaHash, - apollo: this.apolloConfig, - serverlessFramework: this.serverlessFramework(), - engine: { - serviceID: this.apolloConfig.graphId, - apiKeyHash: this.apolloConfig.keyHash, - }, - }; - - // The `persistedQueries` attribute on the GraphQLServiceContext was - // originally used by the operation registry, which shared the cache with - // it. This is no longer the case. However, while we are continuing to - // expand the support of the interface for `persistedQueries`, e.g. with - // additions like https://github.com/apollographql/apollo-server/pull/3623, - // we don't want to continually expand the API surface of what we expose - // to the plugin API. In this particular case, it certainly doesn't need - // to get the `ttl` default value which are intended for APQ only. - if (this.requestOptions.persistedQueries?.cache) { - service.persistedQueries = { - cache: this.requestOptions.persistedQueries.cache, - }; + // Two parallel calls to stop; just wait for the other one to finish and + // do whatever it did. + if (this.state.phase === 'stopping') { + await this.state.barrier; + // The cast here is because TS doesn't understand that this.state can + // change during the await + // (https://github.com/microsoft/TypeScript/issues/9998). + const state = this.state as ServerState; + if (state.phase !== 'stopped') { + throw Error(`Surprising post-stopping state ${state.phase}`); + } + if (state.stopError) { + throw state.stopError; + } + return; } - 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() { - // We run shutdown handlers in two phases because we don't want to turn - // off our signal listeners until we've done the important parts of shutdown - // like running serverWillStop handlers. (We can make this more generic later - // if it's helpful.) - await Promise.all([...this.toDispose].map(dispose => dispose())); - if (this.subscriptionServer) this.subscriptionServer.close(); - await Promise.all([...this.toDisposeLast].map(dispose => dispose())); + // Commit to stopping, actually stop, and update the phase. + this.state = { phase: 'stopping', barrier: resolvable() }; + try { + // We run shutdown handlers in two phases because we don't want to turn + // off our signal listeners until we've done the important parts of shutdown + // like running serverWillStop handlers. (We can make this more generic later + // if it's helpful.) + await Promise.all([...this.toDispose].map((dispose) => dispose())); + if (this.subscriptionServer) this.subscriptionServer.close(); + await Promise.all([...this.toDisposeLast].map((dispose) => dispose())); + } catch (stopError) { + this.state = { phase: 'stopped', stopError }; + return; + } + this.state = { phase: 'stopped', stopError: null }; } - public installSubscriptionHandlers(server: HttpServer | HttpsServer | Http2Server | Http2SecureServer | WebSocket.Server) { + public installSubscriptionHandlers( + server: + | HttpServer + | HttpsServer + | Http2Server + | Http2SecureServer + | WebSocket.Server, + ) { if (!this.subscriptionServerOptions) { if (this.config.gateway) { throw Error( @@ -650,12 +875,30 @@ export class ApolloServerBase { path, } = this.subscriptionServerOptions; - // TODO: This shouldn't use this.schema, as it is deprecated in favor of the schemaDerivedData promise. - const schema = this.schema; - if (this.schema === undefined) - throw new Error( - 'Schema undefined during creation of subscription server.', - ); + let schema: GraphQLSchema; + switch (this.state.phase) { + case 'initialized with schema': + case 'invoking serverWillStart': + case 'started': + schema = this.state.schemaDerivedData.schema; + break; + case 'initialized with gateway': + // shouldn't happen: gateway doesn't support subs + case 'starting': + // shouldn't happen: there's no await between 'starting' and + // 'invoking serverWillStart' without gateway + case 'failed to start': + // only happens if you call 'start' yourself, in which case you really + // ought to see what happens before calling this function + case 'stopping': + case 'stopped': + // stopping is unlikely to happen during startup + throw new Error( + `Can't install subscription handlers when state is ${this.state.phase}`, + ); + default: + throw new UnreachableCaseError(this.state); + } this.subscriptionServer = SubscriptionServer.create( { @@ -699,14 +942,14 @@ export class ApolloServerBase { return { ...connection, context }; }, keepAlive, - validationRules: this.requestOptions.validationRules + validationRules: this.requestOptions.validationRules, }, server instanceof NetServer || server instanceof TlsServer ? { - server, - path, - } - : server + server, + path, + } + : server, ); } @@ -722,38 +965,6 @@ export class ApolloServerBase { return false; } - // Returns true if it appears that the schema was returned from - // @apollo/federation's buildFederatedSchema. This strategy avoids depending - // explicitly on @apollo/federation or relying on something that might not - // survive transformations like monkey-patching a boolean field onto the - // schema. - // - // This is used for two things: - // 1) Determining whether traces should be added to responses if requested - // with an HTTP header. If you want to include these traces even for - // non-federated schemas (when requested via header) you can use - // ApolloServerPluginInlineTrace yourself; if you want to never - // include these traces even for federated schemas you can use - // ApolloServerPluginInlineTraceDisabled. - // 2) Determining whether schema-reporting should be allowed; federated - // services shouldn't be reporting schemas, and we accordingly throw if - // it's attempted. - private schemaIsFederated(schema: GraphQLSchema): boolean { - const serviceType = schema.getType('_Service'); - if (!(serviceType && isObjectType(serviceType))) { - return false; - } - const sdlField = serviceType.getFields().sdl; - if (!sdlField) { - return false; - } - const sdlFieldType = sdlField.type; - if (!isScalarType(sdlFieldType)) { - return false; - } - return sdlFieldType.name == 'String'; - } - private ensurePluginInstantiation(plugins: PluginDefinition[] = []): void { const pluginsToInit: PluginDefinition[] = []; @@ -769,7 +980,7 @@ export class ApolloServerBase { // inline tracing plugin may be what you want, or just usage reporting if // the goal is to get traces to Apollo's servers.) if (this.config.tracing) { - pluginsToInit.push(pluginTracing()) + pluginsToInit.push(pluginTracing()); } // Enable cache control unless it was explicitly disabled. @@ -801,11 +1012,9 @@ export class ApolloServerBase { pluginsToInit.push(pluginCacheControl(cacheControlOptions)); } - const federatedSchema = this.schema && this.schemaIsFederated(this.schema); - pluginsToInit.push(...plugins); - this.plugins = pluginsToInit.map(plugin => { + this.plugins = pluginsToInit.map((plugin) => { if (typeof plugin === 'function') { return plugin(); } @@ -857,23 +1066,13 @@ export class ApolloServerBase { typeof engine === 'object' && (engine.reportSchema || engine.experimental_schemaReporting); if (alreadyHavePlugin || enabledViaEnvVar || enabledViaLegacyOption) { - if (federatedSchema) { - throw Error( - [ - 'Schema reporting is not yet compatible with federated services.', - "If you're interested in using schema reporting with federated", - 'services, please contact Apollo support. To set up managed federation, see', - 'https://go.apollo.dev/s/managed-federation' - ].join(' '), - ); - } if (this.config.gateway) { throw new Error( [ "Schema reporting is not yet compatible with the gateway. If you're", 'interested in using schema reporting with the gateway, please', 'contact Apollo support. To set up managed federation, see', - 'https://go.apollo.dev/s/managed-federation' + 'https://go.apollo.dev/s/managed-federation', ].join(' '), ); } @@ -929,17 +1128,15 @@ export class ApolloServerBase { 'https://go.apollo.dev/s/migration-engine-plugins', ); } - } else if (federatedSchema && this.config.engine !== false) { - // If we have a federated schema, and we haven't explicitly disabled inline - // tracing via ApolloServerPluginInlineTraceDisabled or engine:false, - // we set up inline tracing. + } else if (this.config.engine !== false) { + // If we haven't explicitly disabled inline tracing via + // ApolloServerPluginInlineTraceDisabled or engine:false, + // we set up inline tracing in "only if federated" mode. // (This is slightly different than the pre-ApolloServerPluginInlineTrace where // we would also avoid doing this if an API key was configured and log a warning.) - this.logger.info( - 'Enabling inline tracing for this federated service. To disable, use ' + - 'ApolloServerPluginInlineTraceDisabled.', - ); - const options: ApolloServerPluginInlineTraceOptions = {}; + const options: ApolloServerPluginInlineTraceOptions = { + __onlyIfSchemaIsFederated: true, + }; if (typeof engine === 'object') { options.rewriteError = engine.rewriteError; } @@ -956,8 +1153,7 @@ export class ApolloServerBase { // for unicode characters, etc.), but it should do a reasonable job at // providing a caching document store for most operations. maxSize: - Math.pow(2, 20) * - (this.experimental_approximateDocumentStoreMiB || 30), + Math.pow(2, 20) * (this.experimental_approximateDocumentStoreMiB || 30), sizeCalculator: approximateObjectSize, }); } @@ -968,12 +1164,24 @@ export class ApolloServerBase { protected async graphQLServerOptions( integrationContextArgument?: Record, ): Promise { - const { - schema, - schemaHash, - documentStore, - extensions, - } = await this.schemaDerivedData; + let schemaDerivedData: SchemaDerivedData; + try { + schemaDerivedData = await this.ensureStarted(); + } catch (err) { + // This means that the user didn't call and await `server.start()` themselves + // (and they're not using `apollo-server` which does that for you). First + // we log the error that prevented startup (which means it will get logged + // once for every GraphQL operation). + this.logStartupError(err); + // Now make the operation itself fail. + // We intentionally do not re-throw actual startup error as it may contain + // implementation details and this error will propagate to the client. + throw new Error( + 'This data graph is missing a valid configuration. More details may be available in the server logs.', + ); + } + + const { schema, schemaHash, documentStore, extensions } = schemaDerivedData; let context: Context = this.context ? this.context : {}; @@ -1026,7 +1234,6 @@ export class ApolloServerBase { options.context = cloneObject(options.context); } - const requestCtx: GraphQLRequestContext = { logger: this.logger, schema: options.schema, diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index ecd00afd2d9..5de0353a247 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -37,16 +37,15 @@ describe('ApolloServerBase construction', () => { it('succeeds when passed a graphVariant in construction', () => { let serverBase; - expect( - () => - new ApolloServerBase({ - typeDefs, - resolvers, - engine: { - graphVariant: 'foo', - apiKey: 'not:real:key', - }, - }).stop() + expect(() => + new ApolloServerBase({ + typeDefs, + resolvers, + engine: { + graphVariant: 'foo', + apiKey: 'not:real:key', + }, + }).stop(), ).not.toThrow(); }); @@ -88,7 +87,7 @@ describe('ApolloServerBase construction', () => { schema: {}, }); }).toThrowErrorMatchingInlineSnapshot( - `"Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible."`, + `"Expected {} to be a GraphQL schema."`, ); }); diff --git a/packages/apollo-server-core/src/plugin/inlineTrace/index.ts b/packages/apollo-server-core/src/plugin/inlineTrace/index.ts index b15772e2c79..2ca056f5a4d 100644 --- a/packages/apollo-server-core/src/plugin/inlineTrace/index.ts +++ b/packages/apollo-server-core/src/plugin/inlineTrace/index.ts @@ -2,6 +2,7 @@ import { Trace } from 'apollo-reporting-protobuf'; import { TraceTreeBuilder } from '../traceTreeBuilder'; import type { ApolloServerPluginUsageReportingOptions } from '../usageReporting/options'; import type { InternalApolloServerPlugin } from '../internalPlugin'; +import { schemaIsFederated } from '../schemaIsFederated'; export interface ApolloServerPluginInlineTraceOptions { /** @@ -11,6 +12,17 @@ export interface ApolloServerPluginInlineTraceOptions { * of the error by modifying it and returning the modified error. */ rewriteError?: ApolloServerPluginUsageReportingOptions['rewriteError']; + /** + * This option is for internal use by `apollo-server-core` only. + * + * By default we want to enable this plugin for federated schemas only, but we + * need to come up with our list of plugins before we have necessarily loaded + * the schema. So (unless the user installs this plugin or + * ApolloServerPluginInlineTraceDisabled themselves), `apollo-server-core` + * always installs this plugin and uses this option to make sure traces are + * only included if the schema appears to be federated. + */ + __onlyIfSchemaIsFederated?: boolean; } // This ftv1 plugin produces a base64'd Trace protobuf containing only the @@ -21,11 +33,31 @@ export interface ApolloServerPluginInlineTraceOptions { export function ApolloServerPluginInlineTrace( options: ApolloServerPluginInlineTraceOptions = Object.create(null), ): InternalApolloServerPlugin { + let enabled: boolean | null = options.__onlyIfSchemaIsFederated ? null : true; return { __internal_plugin_id__() { return 'InlineTrace'; }, + serverWillStart({ schema, logger }) { + // Handle the case that the plugin was implicitly installed. We only want it + // to actually be active if the schema appears to be federated. If you don't + // like the log line, just install `ApolloServerPluginInlineTrace()` in + // `plugins` yourself. + if (enabled === null) { + enabled = schemaIsFederated(schema); + if (enabled) { + logger.info( + 'Enabling inline tracing for this federated service. To disable, use ' + + 'ApolloServerPluginInlineTraceDisabled.', + ); + } + } + }, requestDidStart({ request: { http } }) { + if (!enabled) { + return; + } + const treeBuilder = new TraceTreeBuilder({ rewriteError: options.rewriteError, }); diff --git a/packages/apollo-server-core/src/plugin/schemaIsFederated.ts b/packages/apollo-server-core/src/plugin/schemaIsFederated.ts new file mode 100644 index 00000000000..4d491720d2b --- /dev/null +++ b/packages/apollo-server-core/src/plugin/schemaIsFederated.ts @@ -0,0 +1,33 @@ +import { GraphQLSchema, isObjectType, isScalarType } from 'graphql'; + +// Returns true if it appears that the schema was returned from +// @apollo/federation's buildFederatedSchema. This strategy avoids depending +// explicitly on @apollo/federation or relying on something that might not +// survive transformations like monkey-patching a boolean field onto the +// schema. +// +// This is used for two things: +// 1) Determining whether traces should be added to responses if requested +// with an HTTP header. If you want to include these traces even for +// non-federated schemas (when requested via header) you can use +// ApolloServerPluginInlineTrace yourself; if you want to never +// include these traces even for federated schemas you can use +// ApolloServerPluginInlineTraceDisabled. +// 2) Determining whether schema-reporting should be allowed; federated +// services shouldn't be reporting schemas, and we accordingly throw if +// it's attempted. +export function schemaIsFederated(schema: GraphQLSchema): boolean { + const serviceType = schema.getType('_Service'); + if (!(serviceType && isObjectType(serviceType))) { + return false; + } + const sdlField = serviceType.getFields().sdl; + if (!sdlField) { + return false; + } + const sdlFieldType = sdlField.type; + if (!isScalarType(sdlFieldType)) { + return false; + } + return sdlFieldType.name == 'String'; +} diff --git a/packages/apollo-server-core/src/plugin/schemaReporting/index.ts b/packages/apollo-server-core/src/plugin/schemaReporting/index.ts index 8501e2484b9..4d7a4806ef1 100644 --- a/packages/apollo-server-core/src/plugin/schemaReporting/index.ts +++ b/packages/apollo-server-core/src/plugin/schemaReporting/index.ts @@ -4,6 +4,7 @@ import { v4 as uuidv4 } from 'uuid'; import { printSchema, validateSchema, buildSchema } from 'graphql'; import { SchemaReporter } from './schemaReporter'; import createSHA from '../../utils/createSHA'; +import { schemaIsFederated } from '../schemaIsFederated'; export interface ApolloServerPluginSchemaReportingOptions { /** @@ -97,6 +98,17 @@ export function ApolloServerPluginSchemaReporting( } } + if (schemaIsFederated(schema)) { + throw Error( + [ + 'Schema reporting is not yet compatible with federated services.', + "If you're interested in using schema reporting with federated", + 'services, please contact Apollo support. To set up managed federation, see', + 'https://go.apollo.dev/s/managed-federation', + ].join(' '), + ); + } + const executableSchema = overrideReportedSchema ?? printSchema(schema); const executableSchemaId = computeExecutableSchemaId(executableSchema); diff --git a/packages/apollo-server-express/README.md b/packages/apollo-server-express/README.md index dc8646f21ac..93f6727be08 100644 --- a/packages/apollo-server-express/README.md +++ b/packages/apollo-server-express/README.md @@ -29,6 +29,7 @@ const resolvers = { }; const server = new ApolloServer({ typeDefs, resolvers }); +// FIXME start const app = express(); server.applyMiddleware({ app }); @@ -69,6 +70,7 @@ const resolvers = { }; const server = new ApolloServer({ typeDefs, resolvers }); +// FIXME start const app = connect(); const path = '/graphql'; diff --git a/packages/apollo-server-express/src/ApolloServer.ts b/packages/apollo-server-express/src/ApolloServer.ts index 9fd6915ab56..8296e3e12b9 100644 --- a/packages/apollo-server-express/src/ApolloServer.ts +++ b/packages/apollo-server-express/src/ApolloServer.ts @@ -122,25 +122,12 @@ export class ApolloServer extends ApolloServerBase { }: GetMiddlewareOptions = {}): express.Router { if (!path) path = '/graphql'; - const router = express.Router(); + // 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(); - // Despite the fact that this `applyMiddleware` function is `async` in - // other integrations (e.g. Hapi), currently it is not for Express (@here). - // That should change in a future version, but that would be a breaking - // change right now (see comment above this method's declaration above). - // - // That said, we do need to await the `willStart` lifecycle event which - // can perform work prior to serving a request. Since Express doesn't - // natively support Promises yet, we'll do this via a middleware that - // calls `next` when the `willStart` finishes. We'll kick off the - // `willStart` right away, so hopefully it'll finish before the first - // request comes in, but we won't call `next` on this middleware until it - // does. (And we'll take care to surface any errors via the `.catch`-able.) - const promiseWillStart = this.willStart(); - - router.use(path, (_req, _res, next) => { - promiseWillStart.then(() => next()).catch(next); - }); + const router = express.Router(); if (!disableHealthCheck) { router.use('/.well-known/apollo/server-health', (req, res) => { diff --git a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts index 5de96fc374a..bced5f2909a 100644 --- a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts @@ -36,8 +36,11 @@ describe('apollo-server-express', () => { let server; let httpServer; testApolloServer( - async options => { + async (options, suppressStartCall?: boolean) => { server = new ApolloServer(options); + if (!suppressStartCall) { + await server.start(); + } const app = express(); server.applyMiddleware({ app }); httpServer = await new Promise(resolve => { @@ -46,8 +49,8 @@ describe('apollo-server-express', () => { return createServerInfo(server, httpServer); }, async () => { - if (server) await server.stop(); if (httpServer && httpServer.listening) await httpServer.close(); + if (server) await server.stop(); }, ); }); diff --git a/packages/apollo-server-fastify/README.md b/packages/apollo-server-fastify/README.md index ce4079df1b9..2846c9dec55 100644 --- a/packages/apollo-server-fastify/README.md +++ b/packages/apollo-server-fastify/README.md @@ -18,6 +18,7 @@ const server = new ApolloServer({ typeDefs, resolvers, }); +// FIXME start const app = require('fastify')(); diff --git a/packages/apollo-server-fastify/src/ApolloServer.ts b/packages/apollo-server-fastify/src/ApolloServer.ts index 42bc458b229..59e3293420a 100644 --- a/packages/apollo-server-fastify/src/ApolloServer.ts +++ b/packages/apollo-server-fastify/src/ApolloServer.ts @@ -85,13 +85,15 @@ export class ApolloServer extends ApolloServerBase { onHealthCheck, }: ServerRegistration = {}) { this.graphqlPath = path ? path : '/graphql'; - 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(); return async ( app: FastifyInstance, ) => { - await promiseWillStart; - if (!disableHealthCheck) { app.get('/.well-known/apollo/server-health', async (req, res) => { // Response follows https://tools.ietf.org/html/draft-inadarei-api-health-check-01 diff --git a/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts index c38730cd47f..b7b2e892162 100644 --- a/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts @@ -37,8 +37,11 @@ describe('apollo-server-fastify', () => { let app: FastifyInstance; testApolloServer( - async options => { + async (options, suppressStartCall?: boolean) => { server = new ApolloServer(options); + if (!suppressStartCall) { + await server.start(); + } app = fastify(); app.register(server.createHandler()); await app.listen(port); diff --git a/packages/apollo-server-fastify/src/__tests__/datasource.test.ts b/packages/apollo-server-fastify/src/__tests__/datasource.test.ts index 65b55c21160..97e3d811a81 100644 --- a/packages/apollo-server-fastify/src/__tests__/datasource.test.ts +++ b/packages/apollo-server-fastify/src/__tests__/datasource.test.ts @@ -60,7 +60,6 @@ restAPI.get('/str/:id', (req, res) => { }); describe('apollo-server-fastify', () => { - let restServer: FastifyInstance; let app: FastifyInstance; beforeAll(async () => { @@ -68,7 +67,7 @@ describe('apollo-server-fastify', () => { }); afterAll(async () => { - await new Promise(resolve => restServer.close(() => resolve())); + await new Promise(resolve => restAPI.close(() => resolve())); }); let server: ApolloServer; @@ -79,7 +78,7 @@ describe('apollo-server-fastify', () => { afterEach(async () => { await server.stop(); - await new Promise(resolve => app.close(() => resolve())); + await new Promise(resolve => app.close(() => resolve())); }); it('uses the cache', async () => { @@ -90,10 +89,11 @@ describe('apollo-server-fastify', () => { id: new IdAPI(), }), }); + await server.start(); app = fastify(); app.register(server.createHandler()); - await app.listen(6667); + await app.listen(0); const { url: uri } = createServerInfo(server, app.server); const apolloFetch = createApolloFetch({ uri }); @@ -118,10 +118,11 @@ describe('apollo-server-fastify', () => { id: new IdAPI(), }), }); + await server.start(); app = fastify(); app.register(server.createHandler()); - await app.listen(6668); + await app.listen(0); const { url: uri } = createServerInfo(server, app.server); const apolloFetch = createApolloFetch({ uri }); diff --git a/packages/apollo-server-hapi/README.md b/packages/apollo-server-hapi/README.md index 33f4347767b..1b172c19cfd 100644 --- a/packages/apollo-server-hapi/README.md +++ b/packages/apollo-server-hapi/README.md @@ -18,6 +18,7 @@ const Hapi = require('hapi'); async function StartServer() { const server = new ApolloServer({ typeDefs, resolvers }); + // FIXME start const app = new Hapi.server({ port: 4000 diff --git a/packages/apollo-server-hapi/src/ApolloServer.ts b/packages/apollo-server-hapi/src/ApolloServer.ts index 0575d89c149..cafd8fc7ed4 100644 --- a/packages/apollo-server-hapi/src/ApolloServer.ts +++ b/packages/apollo-server-hapi/src/ApolloServer.ts @@ -60,7 +60,10 @@ export class ApolloServer extends ApolloServerBase { disableHealthCheck, onHealthCheck, }: ServerRegistration) { - 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(); if (!path) path = '/graphql'; diff --git a/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts index 7699878bcbc..38fb11f40c6 100644 --- a/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts @@ -27,9 +27,12 @@ const port = 0; const { Server } = require('hapi'); testApolloServer( - async options => { + async (options, suppressStartCall?: boolean) => { server = new ApolloServer(options); app = new Server({ host: 'localhost', port }); + if (!suppressStartCall) { + await server.start(); + } await server.applyMiddleware({ app }); await app.start(); const httpServer = app.listener; diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 295b54e0b23..706af759511 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -45,6 +45,7 @@ import { ApolloServerPluginInlineTrace, ApolloServerPluginUsageReporting, ApolloServerPluginUsageReportingOptions, + GraphQLServiceConfig, } from 'apollo-server-core'; import { GraphQLExtension, GraphQLResponse } from 'graphql-extensions'; import { TracingFormat } from 'apollo-tracing'; @@ -123,28 +124,31 @@ const makeGatewayMock = ({ unsubscribeSpy?: () => void; executor?: GraphQLExecutor; } = {}) => { + let resolution: GraphQLServiceConfig | null = null; + let rejection: Error | null = null; const eventuallyAssigned = { - resolveLoad: null as ({ schema, executor }) => void, - rejectLoad: null as (err: Error) => void, + resolveLoad: (config: GraphQLServiceConfig) => { + resolution = config; + }, + rejectLoad: (err: Error) => { + rejection = err; + }, triggerSchemaChange: null as (newSchema) => void, }; - const mockedLoadResults = new Promise<{ - schema: GraphQLSchema; - executor: GraphQLExecutor; - }>((resolve, reject) => { - eventuallyAssigned.resolveLoad = ({ schema, executor }) => { - resolve({ schema, executor }); - }; - eventuallyAssigned.rejectLoad = (err: Error) => { - reject(err); - }; - }); const mockedGateway: GraphQLService = { executor, - load: options => { + load: async (options) => { optionsSpy(options); - return mockedLoadResults; + // Make sure it's async + await new Promise(res => setImmediate(res)); + if (rejection) { + throw rejection; + } + if (resolution) { + return resolution; + } + throw Error("Neither resolving nor rejecting?"); }, onSchemaChange: callback => { eventuallyAssigned.triggerSchemaChange = callback; @@ -165,7 +169,7 @@ export interface ServerInfo { } export interface CreateServerFunc { - (config: Config): Promise>; + (config: Config, suppressStartCall?: boolean): Promise>; } export interface StopServerFunc { @@ -387,6 +391,18 @@ export function testApolloServer( expect(executor).toHaveBeenCalled(); }); + it("rejected load promise is thrown by server.start", async () => { + const { gateway, triggers } = makeGatewayMock(); + + const loadError = new Error("load error which should be be thrown by start"); + triggers.rejectLoad(loadError); + + expect(createApolloServer({ + gateway, + subscriptions: false, + })).rejects.toThrowError(loadError); + }); + it("rejected load promise acts as an error boundary", async () => { const executor = jest.fn(); executor.mockResolvedValueOnce( @@ -407,7 +423,7 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ gateway, subscriptions: false, - }); + }, true); const apolloFetch = createApolloFetch({ uri }); const result = await apolloFetch({ query: '{testString}' }); @@ -423,8 +439,13 @@ export function testApolloServer( }) ); expect(consoleErrorSpy).toHaveBeenCalledWith( - "This data graph is missing a valid configuration. " + - "load error which should be masked"); + 'Apollo Server was started implicitly and an error occurred during startup. ' + + '(Consider calling `await server.start()` immediately after ' + + '`server = new ApolloServer()` so you can handle these errors directly before ' + + 'starting your web server.) All GraphQL requests will now fail. The startup error ' + + 'was: ' + + 'load error which should be masked', + ); expect(executor).not.toHaveBeenCalled(); }); @@ -2673,7 +2694,7 @@ export function testApolloServer( }); it('reports a total duration that is longer than the duration of its resolvers', async () => { - const { url: uri } = await createApolloServer({ + const { url: uri, server } = await createApolloServer({ typeDefs: allTypeDefs, resolvers, }); @@ -3279,7 +3300,7 @@ export function testApolloServer( const { gateway, triggers } = makeGatewayMock({ optionsSpy }); triggers.resolveLoad({ schema, executor: () => {} }); - await createApolloServer({ + const { server } = await createApolloServer({ gateway, subscriptions: false, apollo: { key: 'service:tester:1234abc', graphVariant: 'staging' }, diff --git a/packages/apollo-server-integration-testsuite/src/index.ts b/packages/apollo-server-integration-testsuite/src/index.ts index e39cb84d036..75e226eed6d 100644 --- a/packages/apollo-server-integration-testsuite/src/index.ts +++ b/packages/apollo-server-integration-testsuite/src/index.ts @@ -17,6 +17,7 @@ import { } from 'graphql'; import request from 'supertest'; +import resolvable from '@josephg/resolvable'; import { GraphQLOptions, @@ -1119,15 +1120,18 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { describe('request pipeline plugins', () => { describe('lifecycle hooks', () => { + // This tests the backwards-compatibility behavior that ensures + // that even if you don't call server.start() yourself, the first + // GraphQL request will result in starting the server before + // serving the first request it('calls serverWillStart before serving a request', async () => { - // We'll use this eventually-assigned function to programmatically - // resolve the `serverWillStart` event. - let resolveServerWillStart: Function; - - // We'll use this mocked function to determine the order in which + // We'll use this to determine the order in which // the events we're expecting to happen actually occur and validate // those expectations in various stages of this test. - const fn = jest.fn(); + const calls: string[] = []; + + const pluginStartedBarrier = resolvable(); + const letPluginFinishBarrier = resolvable(); // We want this to create the app as fast as `createApp` will allow. // for integrations whose `applyMiddleware` currently returns a @@ -1138,35 +1142,17 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { schema, plugins: [ { - serverWillStart() { - fn('zero'); - return new Promise(resolve => { - resolveServerWillStart = () => { - fn('one'); - resolve(); - }; - }); + async serverWillStart() { + calls.push('zero'); + pluginStartedBarrier.resolve(); + await letPluginFinishBarrier; + calls.push('one'); }, }, ], }, }); - const delayUntil = async (check: () => boolean, expectedNumTicks) => { - if (check()) return expect(expectedNumTicks).toBe(0); - else expect(expectedNumTicks).not.toBe(0); - return new Promise(resolve => - process.nextTick(() => - delayUntil(check, expectedNumTicks - 1).then(resolve), - ), - ); - }; - - // Make sure that things were called in the expected order. - await delayUntil(() => fn.mock.calls.length === 1, 1); - expect(fn.mock.calls).toEqual([['zero']]); - resolveServerWillStart(); - // Account for the fact that `createApp` might return a Promise, // and might not, depending on the integration's implementation of // createApp. This is entirely to account for the fact that @@ -1185,18 +1171,22 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { query: 'query test{ testString }', }) .then(res => { - fn('two'); + calls.push('two'); return res; }); - // Ensure the request has not gone through. - expect(fn.mock.calls).toEqual([['zero'], ['one']]); + // At this point calls might be [] or ['zero'] because the back-compat + // code kicks off start() asynchronously. We can safely wait on + // the plugin's serverWillStart to begin. + await pluginStartedBarrier; + expect(calls).toEqual(['zero']); + letPluginFinishBarrier.resolve(); // Now, wait for the request to finish. await res; // Finally, ensure that the order we expected was achieved. - expect(fn.mock.calls).toEqual([['zero'], ['one'], ['two']]); + expect(calls).toEqual(['zero', 'one', 'two']); }); }); }); diff --git a/packages/apollo-server-koa/README.md b/packages/apollo-server-koa/README.md index ec416cf77c2..54929c74c6d 100644 --- a/packages/apollo-server-koa/README.md +++ b/packages/apollo-server-koa/README.md @@ -29,6 +29,7 @@ const resolvers = { }; const server = new ApolloServer({ typeDefs, resolvers }); +// FIXME start const app = new Koa(); server.applyMiddleware({ app }); diff --git a/packages/apollo-server-koa/src/ApolloServer.ts b/packages/apollo-server-koa/src/ApolloServer.ts index 2043e299f60..3608744b52c 100644 --- a/packages/apollo-server-koa/src/ApolloServer.ts +++ b/packages/apollo-server-koa/src/ApolloServer.ts @@ -101,26 +101,12 @@ export class ApolloServer extends ApolloServerBase { }: GetMiddlewareOptions = {}): Middleware { if (!path) path = '/graphql'; - // Despite the fact that this `applyMiddleware` function is `async` in - // other integrations (e.g. Hapi), currently it is not for Koa (@here). - // That should change in a future version, but that would be a breaking - // change right now (see comment above this method's declaration above). - // - // That said, we do need to await the `willStart` lifecycle event which - // can perform work prior to serving a request. While we could do this - // via awaiting in a Koa middleware, well kick off `willStart` right away, - // so hopefully it'll finish before the first request comes in. We won't - // call `next` until it's ready, which will effectively yield until that - // work has finished. Any errors will be surfaced to Koa through its own - // native Promise-catching facilities. - 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 middlewares = []; - middlewares.push( - middlewareFromPath(path, async (_ctx: Koa.Context, next: Function) => { - await promiseWillStart; - return next(); - }), - ); if (!disableHealthCheck) { middlewares.push( diff --git a/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts index af8374b9e59..c5a4995e3d8 100644 --- a/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts @@ -41,8 +41,11 @@ const resolvers = { let server: ApolloServer; let httpServer: http.Server; testApolloServer( - async options => { + async (options, suppressStartCall?: boolean) => { server = new ApolloServer(options); + if (!suppressStartCall) { + await server.start(); + } const app = new Koa(); server.applyMiddleware({ app }); httpServer = await new Promise(resolve => { diff --git a/packages/apollo-server-lambda/README.md b/packages/apollo-server-lambda/README.md index 85871317ead..ccae3a54719 100644 --- a/packages/apollo-server-lambda/README.md +++ b/packages/apollo-server-lambda/README.md @@ -36,7 +36,7 @@ const resolvers = { const server = new ApolloServer({ typeDefs, resolvers, - + // By default, the GraphQL Playground interface and GraphQL introspection // is disabled in "production" (i.e. when `process.env.NODE_ENV` is `production`). // @@ -45,6 +45,7 @@ const server = new ApolloServer({ playground: true, introspection: true, }); +// FIXME start exports.handler = server.createHandler(); ``` @@ -135,6 +136,7 @@ const server = new ApolloServer({ context, }), }); +// FIXME start exports.handler = server.createHandler(); ``` @@ -164,6 +166,7 @@ const server = new ApolloServer({ typeDefs, resolvers, }); +// FIXME start exports.handler = server.createHandler({ cors: { @@ -196,6 +199,7 @@ const server = new ApolloServer({ typeDefs, resolvers, }); +// FIXME start exports.handler = server.createHandler({ cors: { diff --git a/packages/apollo-server-lambda/src/ApolloServer.ts b/packages/apollo-server-lambda/src/ApolloServer.ts index 1798da88b9b..d027eff2322 100644 --- a/packages/apollo-server-lambda/src/ApolloServer.ts +++ b/packages/apollo-server-lambda/src/ApolloServer.ts @@ -108,11 +108,6 @@ export class ApolloServer extends ApolloServerBase { onHealthCheck: 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(); - const corsHeaders = new Headers(); if (cors) { @@ -340,14 +335,6 @@ export class ApolloServer extends ApolloServerBase { { method: event.httpMethod, options: 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(event, context); }, query, diff --git a/packages/apollo-server-micro/README.md b/packages/apollo-server-micro/README.md index a4197b85db3..6acb2c7f25d 100644 --- a/packages/apollo-server-micro/README.md +++ b/packages/apollo-server-micro/README.md @@ -35,6 +35,7 @@ const resolvers = { }; const apolloServer = new ApolloServer({ typeDefs, resolvers }); +// FIXME start module.exports = apolloServer.createHandler(); ``` @@ -86,6 +87,7 @@ const resolvers = { }; const apolloServer = new ApolloServer({ typeDefs, resolvers }); +// FIXME start const handler = apolloServer.createHandler(); // highlight-line module.exports = cors((req, res) => req.method === 'OPTIONS' ? res.end() : handler(req, res)) // highlight-line ``` @@ -137,6 +139,7 @@ const resolvers = { }; const apolloServer = new ApolloServer({ typeDefs, resolvers }); +// FIXME start module.exports = apolloServer.createHandler({ path: '/data' }); // highlight-line ``` @@ -189,6 +192,7 @@ const resolvers = { }; const apolloServer = new ApolloServer({ typeDefs, resolvers }); +// FIXME start const graphqlPath = '/data'; const graphqlHandler = apolloServer.createHandler({ path: graphqlPath }); module.exports = router( diff --git a/packages/apollo-server-micro/src/ApolloServer.ts b/packages/apollo-server-micro/src/ApolloServer.ts index 6df712e063e..94b5d605309 100644 --- a/packages/apollo-server-micro/src/ApolloServer.ts +++ b/packages/apollo-server-micro/src/ApolloServer.ts @@ -33,15 +33,14 @@ export class ApolloServer extends ApolloServerBase { disableHealthCheck, onHealthCheck, }: ServerRegistration = {}) { - // We'll kick off the `willStart` right away, so hopefully it'll finish - // before the first request comes in. - 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(); return async (req, res) => { this.graphqlPath = path || '/graphql'; - await promiseWillStart; - if (typeof processFileUploads === 'function') { await this.handleFileUploads(req, res); } diff --git a/packages/apollo-server-plugin-operation-registry/src/__tests__/ApolloServerPluginOperationRegistry.test.ts b/packages/apollo-server-plugin-operation-registry/src/__tests__/ApolloServerPluginOperationRegistry.test.ts index c2c2451d730..3512f4a6be3 100644 --- a/packages/apollo-server-plugin-operation-registry/src/__tests__/ApolloServerPluginOperationRegistry.test.ts +++ b/packages/apollo-server-plugin-operation-registry/src/__tests__/ApolloServerPluginOperationRegistry.test.ts @@ -24,6 +24,7 @@ import { } from './helpers.test-helpers'; import { Headers } from 'apollo-server-env'; import { GraphQLRequest } from 'apollo-server-plugin-base'; +import { ApolloConfigInput } from 'apollo-server-types'; // While not ideal, today, Apollo Server has a very real expectation of an HTTP // request context. That will change in the future. While we can sometimes @@ -76,13 +77,6 @@ describe('Operation registry plugin', () => { ); const queryHash = operationSignature(normalizedQueryDocument); - // In order to expose will start and - class ApolloServerMock extends ApolloServerBase { - public async willStart() { - return super.willStart(); - } - } - describe('onUnregisterOperation', () => { it('is called when unregistered operation received', async () => { const onUnregisteredOperation: Options['onUnregisteredOperation'] = jest.fn(); @@ -92,7 +86,7 @@ describe('Operation registry plugin', () => { genericStorageSecret, [ /* Intentionally empty! */ ], ); - const server = new ApolloServerMock({ + const server = new ApolloServerBase({ typeDefs, mockEntireSchema: true, apollo, @@ -103,7 +97,7 @@ describe('Operation registry plugin', () => { })(), ], }); - await server.willStart(); + await server.start(); const result = await server.executeOperation({ ...mockHttpRequestContextForExecuteOperation, query: print(query), @@ -143,7 +137,7 @@ describe('Operation registry plugin', () => { }, ], ); - const server = new ApolloServerMock({ + const server = new ApolloServerBase({ typeDefs, mockEntireSchema: true, apollo, @@ -154,7 +148,7 @@ describe('Operation registry plugin', () => { })(), ], }); - await server.willStart(); + await server.start(); const result = await server.executeOperation({ ...mockHttpRequestContextForExecuteOperation, query: print(query), @@ -180,7 +174,7 @@ describe('Operation registry plugin', () => { genericStorageSecret, [ /* Intentionally empty! */ ], ); - const server = new ApolloServerMock({ + const server = new ApolloServerBase({ typeDefs, mockEntireSchema: true, apollo, @@ -192,7 +186,7 @@ describe('Operation registry plugin', () => { })(), ], }); - await server.willStart(); + await server.start(); const result = await server.executeOperation({ ...mockHttpRequestContextForExecuteOperation, query: print(query), @@ -234,7 +228,7 @@ describe('Operation registry plugin', () => { genericStorageSecret, [ /* Intentionally empty! */ ], ); - const server = new ApolloServerMock({ + const server = new ApolloServerBase({ typeDefs, mockEntireSchema: true, apollo, @@ -246,7 +240,7 @@ describe('Operation registry plugin', () => { })(), ], }); - await server.willStart(); + await server.start(); const result = await server.executeOperation({ ...mockHttpRequestContextForExecuteOperation, query: print(query), @@ -273,7 +267,7 @@ describe('Operation registry plugin', () => { }, ], ); - const server = new ApolloServerMock({ + const server = new ApolloServerBase({ typeDefs, mockEntireSchema: true, apollo, @@ -284,7 +278,7 @@ describe('Operation registry plugin', () => { })(), ], }); - await server.willStart(); + await server.start(); const result = await server.executeOperation({ ...mockHttpRequestContextForExecuteOperation, query: print(query), diff --git a/packages/apollo-server/src/__tests__/index.test.ts b/packages/apollo-server/src/__tests__/index.test.ts index 94e28648936..8f4d9830c1f 100644 --- a/packages/apollo-server/src/__tests__/index.test.ts +++ b/packages/apollo-server/src/__tests__/index.test.ts @@ -1,6 +1,7 @@ import { createConnection } from 'net'; import request from 'request'; import { createApolloFetch } from 'apollo-fetch'; +import resolvable from '@josephg/resolvable'; import { gql, ApolloServer } from '../index'; @@ -17,19 +18,6 @@ const resolvers = { }, }; -class Barrier { - private resolvePromise!: () => void; - private promise = new Promise((r) => { - this.resolvePromise = r; - }); - async wait() { - await this.promise; - } - unblock() { - this.resolvePromise(); - } -} - describe('apollo-server', () => { describe('constructor', () => { it('accepts typeDefs and resolvers', () => { @@ -82,11 +70,11 @@ describe('apollo-server', () => { // Open a TCP connection to the server, and let it dangle idle // without starting a request. - const connectionBarrier = new Barrier(); + const connectionBarrier = resolvable(); createConnection({ host: 'localhost', port: port as number }, () => - connectionBarrier.unblock(), + connectionBarrier.resolve(), ); - await connectionBarrier.wait(); + await connectionBarrier; // Stop the server. Before, when this was just net.Server.close, this // would hang. Now that we use stoppable, the idle connection is immediately @@ -95,8 +83,8 @@ describe('apollo-server', () => { }); it('a connection with an active HTTP request', async () => { - const gotToHangBarrier = new Barrier(); - const hangBarrier = new Barrier(); + const gotToHangBarrier = resolvable(); + const hangBarrier = resolvable(); const server = new ApolloServer({ typeDefs, resolvers: { @@ -104,8 +92,8 @@ describe('apollo-server', () => { Query: { ...resolvers.Query, async hang() { - gotToHangBarrier.unblock(); - await hangBarrier.wait(); // never unblocks + gotToHangBarrier.resolve(); + await hangBarrier; // never unblocks }, }, }, @@ -119,7 +107,7 @@ describe('apollo-server', () => { // expected error that happens after the server is stopped.) const apolloFetch = createApolloFetch({ uri: url }); apolloFetch({ query: '{hang}' }).catch(() => {}); - await gotToHangBarrier.wait(); + await gotToHangBarrier; // Stop the server. Before, when this was just net.Server.close, this // would hang. Now that we use stoppable, the idle connection is immediately diff --git a/packages/apollo-server/src/index.ts b/packages/apollo-server/src/index.ts index cafe3e49df9..ebe40db0b0e 100644 --- a/packages/apollo-server/src/index.ts +++ b/packages/apollo-server/src/index.ts @@ -95,8 +95,18 @@ export class ApolloServer extends ApolloServerBase { ); } + public async start(): Promise { + throw new Error( + "When using the `apollo-server` package, you don't need to call start(); just call listen().", + ); + } + // Listen takes the same arguments as http.Server.listen. public async listen(...opts: Array): Promise { + // First start the server and throw if startup fails (eg, schema can't be loaded + // or a serverWillStart plugin throws). + await this._start(); + // This class is the easy mode for people who don't create their own express // object, so we have to create it. const app = express();