From 28791efd8df7e63f4853dc74226d34d64b97aede Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 4 Aug 2020 01:02:18 -0700 Subject: [PATCH] Add serverWillStop lifecycle hook; call stop() on signals by default Fixes #4273. This PR adds a serverWillStop plugin lifecycle hook. The `serverWillStop` hook is on an object optionally returned from a `serverWillStart` hook, similar to `executionDidStart`/`executionDidEnd`. ApolloServerPluginOperationRegistry uses this to stop its agent. The code that installs SIGINT and SIGTERM handlers unless disabled with `handleSignals: false` is hoisted from EngineReportingAgent to ApolloServer itself and renamed to `stopOnTerminationSignals` as a new ApolloServer option. The new implementation also skips installing the signals handlers by default if NODE_ENV=test or if you don't appear to be running in Node (and we update some tests that explicitly set other NODE_ENVs to set handleSignals: false). The main effect on existing code is that on one of these signals, any SubscriptionServer and ApolloGateway will be stopped in addition to any EngineReportingAgent. (This commit adds new documentation for `stopOnTerminationSignals` to the API reference but does not change the documentation of `EngineReportingOptions.handleSignals` on that page because a later commit in this PR removes that section entirely.) --- docs/source/api/apollo-server.md | 21 +++++++ packages/apollo-engine-reporting/src/agent.ts | 31 +--------- .../apollo-server-core/src/ApolloServer.ts | 60 +++++++++++++++---- packages/apollo-server-core/src/types.ts | 1 + .../src/utils/pluginTestHarness.ts | 10 +++- .../src/__tests__/ApolloServer.test.ts | 6 +- .../src/__tests__/ApolloServer.test.ts | 2 +- .../src/__tests__/ApolloServer.test.ts | 5 ++ .../src/ApolloServer.ts | 8 +++ .../src/__tests__/ApolloServer.test.ts | 2 +- .../src/__tests__/ApolloServer.test.ts | 6 +- .../apollo-server-plugin-base/src/index.ts | 10 +++- .../ApolloServerPluginOperationRegistry.ts | 9 ++- .../apollo-server/src/__tests__/index.test.ts | 30 ++++++++++ 14 files changed, 150 insertions(+), 51 deletions(-) diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 9618d8aeeaf..208f4a9ef97 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -289,6 +289,27 @@ Provide this function to transform the structure of GraphQL response objects bef +**Lifecycle options** + + + + + + +###### `stopOnTerminationSignals` + +`Boolean` + + + +By default (when running in Node and when the `NODE_ENV` environment variable does not equal `test`), ApolloServer listens for the `SIGINT` and `SIGTERM` signals and calls `await this.stop()` on itself when it is received, and then re-sends the signal to itself so that process shutdown can continue. Set this to false to disable this behavior, or to true to enable this behavior even when `NODE_ENV` is `test`. You can manually invoke `stop()` in other contexts if you'd like. Note that `stop()` does not run synchronously so it cannot work usefully in an `process.on('exit')` handler. + + + + + + + **Debugging options** diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index cca8cb0c4bf..5105bfbcd93 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -290,11 +290,8 @@ export interface EngineReportingOptions { */ privateHeaders?: Array | boolean; /** - * By default, EngineReportingAgent listens for the 'SIGINT' and 'SIGTERM' - * signals, stops, sends a final report, and re-sends the signal to - * itself. Set this to false to disable. You can manually invoke 'stop()' and - * 'sendReport()' on other signals if you'd like. Note that 'sendReport()' - * does not run synchronously so it cannot work usefully in an 'exit' handler. + * For backwards compatibility only; specifying `new ApolloServer({engine: {handleSignals: false}})` is + * equivalent to specifying `new ApolloServer({stopOnTerminationSignals: false})`. */ handleSignals?: boolean; /** @@ -445,8 +442,6 @@ export class EngineReportingAgent { private stopped: boolean = false; private signatureCache: InMemoryLRUCache; - private signalHandlers = new Map(); - private currentSchemaReporter?: SchemaReporter; private readonly bootId: string; private lastSeenExecutableSchemaToId?: { @@ -529,21 +524,6 @@ export class EngineReportingAgent { ); } - if (this.options.handleSignals !== false) { - const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM']; - signals.forEach(signal => { - // Note: Node only started sending signal names to signal events with - // Node v10 so we can't use that feature here. - const handler: NodeJS.SignalsListener = async () => { - this.stop(); - await this.sendAllReportsAndReportErrors(); - process.kill(process.pid, signal); - }; - process.once(signal, handler); - this.signalHandlers.set(signal, handler); - }); - } - if (this.options.endpointUrl) { this.logger.warn( '[deprecated] The `endpointUrl` option within `engine` has been renamed to `tracesEndpointUrl`.', @@ -847,11 +827,6 @@ export class EngineReportingAgent { // size, and stop buffering new traces. You may still manually send a last // report by calling sendReport(). public stop() { - // Clean up signal handlers so they don't accrue indefinitely. - this.signalHandlers.forEach((handler, signal) => { - process.removeListener(signal, handler); - }); - if (this.reportTimer) { clearInterval(this.reportTimer); this.reportTimer = undefined; @@ -930,7 +905,7 @@ export class EngineReportingAgent { return generatedSignature; } - private async sendAllReportsAndReportErrors(): Promise { + public async sendAllReportsAndReportErrors(): Promise { await Promise.all( Object.keys(this.reportDataByExecutableSchemaId).map(executableSchemaId => this.sendReportAndReportErrors(executableSchemaId), diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index c903c9722ee..eda7a85adce 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -31,6 +31,7 @@ import { import { ApolloServerPlugin, GraphQLServiceContext, + GraphQLServerListener, } from 'apollo-server-plugin-base'; import runtimeSupportsUploads from './utils/runtimeSupportsUploads'; @@ -76,13 +77,14 @@ import { import { Headers } from 'apollo-server-env'; import { buildServiceDefinition } from '@apollographql/apollo-tools'; import { plugin as pluginTracing } from "apollo-tracing"; -import { Logger, SchemaHash } from "apollo-server-types"; +import { Logger, SchemaHash, ValueOrPromise } from "apollo-server-types"; import { plugin as pluginCacheControl, CacheControlExtensionOptions, } from 'apollo-cache-control'; import { getEngineApiKey, getEngineGraphVariant } from "apollo-engine-reporting/dist/agent"; import { cloneObject } from "./runHttpQuery"; +import isNodeLike from './utils/isNodeLike'; const NoIntrospection = (context: ValidationContext) => ({ Field(node: FieldDefinitionNode) { @@ -149,7 +151,7 @@ export class ApolloServerBase { private config: Config; /** @deprecated: This is undefined for servers operating as gateways, and will be removed in a future release **/ protected schema?: GraphQLSchema; - private toDispose = new Set<() => void>(); + private toDispose = new Set<() => ValueOrPromise>(); private experimental_approximateDocumentStoreMiB: Config['experimental_approximateDocumentStoreMiB']; @@ -177,6 +179,7 @@ export class ApolloServerBase { gateway, cacheControl, experimental_approximateDocumentStoreMiB, + stopOnTerminationSignals, ...requestOptions } = config; @@ -385,6 +388,32 @@ export class ApolloServerBase { // is populated accordingly. this.ensurePluginInstantiation(plugins); + // We handle signals if it was explicitly requested, or if we're in Node, + // not in a test, and it wasn't explicitly turned off. (For backwards + // compatibility, we check both 'stopOnTerminationSignals' and + // 'engine.handleSignals'.) + if ( + typeof stopOnTerminationSignals === 'boolean' + ? stopOnTerminationSignals + : typeof this.config.engine === 'object' && + typeof this.config.engine.handleSignals === 'boolean' + ? this.config.engine.handleSignals + : isNodeLike && process.env.NODE_ENV !== 'test' + ) { + const signals: NodeJS.Signals[] = ['SIGINT', 'SIGTERM']; + signals.forEach((signal) => { + // Note: Node only started sending signal names to signal events with + // Node v10 so we can't use that feature here. + const handler: NodeJS.SignalsListener = async () => { + await this.stop(); + process.kill(process.pid, signal); + }; + process.once(signal, handler); + this.toDispose.add(() => { + process.removeListener(signal, handler); + }); + }); + } } // used by integrations to synchronize the path with subscriptions, some @@ -585,24 +614,33 @@ export class ApolloServerBase { if (this.requestOptions.persistedQueries?.cache) { service.persistedQueries = { cache: this.requestOptions.persistedQueries.cache, - } + }; } - await Promise.all( - this.plugins.map( - plugin => - plugin.serverWillStart && - plugin.serverWillStart(service), - ), + const serverListeners = ( + await Promise.all( + this.plugins.map( + (plugin) => plugin.serverWillStart && plugin.serverWillStart(service), + ), + ) + ).filter( + (maybeServerListener): maybeServerListener is GraphQLServerListener => + typeof maybeServerListener === 'object' && + !!maybeServerListener.serverWillStop, ); + this.toDispose.add(async () => { + await Promise.all( + serverListeners.map(({ serverWillStop }) => serverWillStop?.()), + ); + }); } public async stop() { - this.toDispose.forEach(dispose => dispose()); + await Promise.all([...this.toDispose].map(dispose => dispose())); if (this.subscriptionServer) await this.subscriptionServer.close(); if (this.engineReportingAgent) { this.engineReportingAgent.stop(); - await this.engineReportingAgent.sendAllReports(); + await this.engineReportingAgent.sendAllReportsAndReportErrors(); } } diff --git a/packages/apollo-server-core/src/types.ts b/packages/apollo-server-core/src/types.ts index 86f24ba434f..491be13340a 100644 --- a/packages/apollo-server-core/src/types.ts +++ b/packages/apollo-server-core/src/types.ts @@ -124,6 +124,7 @@ export interface Config extends BaseConfig { playground?: PlaygroundConfig; gateway?: GraphQLService; experimental_approximateDocumentStoreMiB?: number; + stopOnTerminationSignals?: boolean; } export interface FileUploadOptions { diff --git a/packages/apollo-server-core/src/utils/pluginTestHarness.ts b/packages/apollo-server-core/src/utils/pluginTestHarness.ts index 9f2d9f51e3d..af034f07e5e 100644 --- a/packages/apollo-server-core/src/utils/pluginTestHarness.ts +++ b/packages/apollo-server-core/src/utils/pluginTestHarness.ts @@ -17,6 +17,7 @@ import { import { ApolloServerPlugin, GraphQLRequestExecutionListener, + GraphQLServerListener, } from 'apollo-server-plugin-base'; import { InMemoryLRUCache } from 'apollo-server-caching'; import { Dispatcher } from './dispatcher'; @@ -98,16 +99,19 @@ export default async function pluginTestHarness({ } const schemaHash = generateSchemaHash(schema); + let serverListener: GraphQLServerListener | undefined; if (typeof pluginInstance.serverWillStart === 'function') { - pluginInstance.serverWillStart({ + const maybeServerListener = await pluginInstance.serverWillStart({ logger: logger || console, schema, schemaHash, engine: {}, }); + if (maybeServerListener && maybeServerListener.serverWillStop) { + serverListener = maybeServerListener; + } } - const requestContext: GraphQLRequestContext = { logger: logger || console, schema, @@ -188,5 +192,7 @@ export default async function pluginTestHarness({ requestContext as GraphQLRequestContextWillSendResponse, ); + await serverListener?.serverWillStop?.(); + return requestContext as GraphQLRequestContextWillSendResponse; } diff --git a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts index 69e5598ef09..b78254c1bdd 100644 --- a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts @@ -62,7 +62,7 @@ describe('apollo-server-express', () => { serverOptions: ApolloServerExpressConfig, options: Partial = {}, ) { - server = new ApolloServer(serverOptions); + server = new ApolloServer({stopOnTerminationSignals: false, ...serverOptions}); app = express(); server.applyMiddleware({ ...options, app }); @@ -184,13 +184,12 @@ describe('apollo-server-express', () => { }); it('renders GraphQL playground using request original url', async () => { - const nodeEnv = process.env.NODE_ENV; - delete process.env.NODE_ENV; const samplePath = '/innerSamplePath'; const rewiredServer = new ApolloServer({ typeDefs, resolvers, + playground: true, }); const innerApp = express(); rewiredServer.applyMiddleware({ app: innerApp }); @@ -218,7 +217,6 @@ describe('apollo-server-express', () => { }, }, (error, response, body) => { - process.env.NODE_ENV = nodeEnv; if (error) { reject(error); } else { diff --git a/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts index b31ea534ace..c38730cd47f 100644 --- a/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-fastify/src/__tests__/ApolloServer.test.ts @@ -64,7 +64,7 @@ describe('apollo-server-fastify', () => { options: Partial = {}, mockDecorators: boolean = false, ) { - server = new ApolloServer(serverOptions); + server = new ApolloServer({ stopOnTerminationSignals: false, ...serverOptions }); app = fastify(); if (mockDecorators) { diff --git a/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts index 2ced8e8c66e..7699878bcbc 100644 --- a/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-hapi/src/__tests__/ApolloServer.test.ts @@ -155,6 +155,7 @@ const port = 0; server = new ApolloServer({ typeDefs, resolvers, + stopOnTerminationSignals: false, }); app = new Server({ port }); @@ -514,6 +515,7 @@ const port = 0; server = new ApolloServer({ typeDefs, resolvers, + stopOnTerminationSignals: false, context: () => { throw new AuthenticationError('valid result'); }, @@ -562,6 +564,7 @@ const port = 0; }, }, }, + stopOnTerminationSignals: false, }); app = new Server({ port }); @@ -609,6 +612,7 @@ const port = 0; }, }, }, + stopOnTerminationSignals: false, }); app = new Server({ port }); @@ -653,6 +657,7 @@ const port = 0; }, }, }, + stopOnTerminationSignals: false, }); app = new Server({ port }); diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts index 98824cf97fb..a1efd8ce159 100644 --- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts +++ b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts @@ -233,6 +233,7 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ schema, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); @@ -250,6 +251,7 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ schema, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); @@ -272,6 +274,7 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ schema, introspection: true, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); @@ -1009,6 +1012,7 @@ export function testApolloServer( }, formatError, debug: true, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); @@ -1078,6 +1082,7 @@ export function testApolloServer( ...engineOptions, }, debug: true, + stopOnTerminationSignals: false, ...constructorOptions, }); @@ -1682,6 +1687,7 @@ export function testApolloServer( const { url: uri } = await createApolloServer({ typeDefs, resolvers, + stopOnTerminationSignals: false, context: () => { throw new AuthenticationError('valid result'); }, @@ -1743,6 +1749,7 @@ export function testApolloServer( }, }, }, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); @@ -1776,6 +1783,7 @@ export function testApolloServer( }, }, }, + stopOnTerminationSignals: false, }); const apolloFetch = createApolloFetch({ uri }); diff --git a/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts index 35d99376164..af8374b9e59 100644 --- a/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-koa/src/__tests__/ApolloServer.test.ts @@ -77,7 +77,7 @@ const resolvers = { serverOptions: Config, options: Partial = {}, ) { - server = new ApolloServer(serverOptions); + server = new ApolloServer({ stopOnTerminationSignals: false, ...serverOptions }); app = new Koa(); server.applyMiddleware({ ...options, app }); diff --git a/packages/apollo-server-micro/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-micro/src/__tests__/ApolloServer.test.ts index 941242f1283..037b9586b88 100644 --- a/packages/apollo-server-micro/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-micro/src/__tests__/ApolloServer.test.ts @@ -22,7 +22,11 @@ const resolvers = { }; async function createServer(options: object = {}): Promise { - const apolloServer = new ApolloServer({ typeDefs, resolvers }); + const apolloServer = new ApolloServer({ + typeDefs, + resolvers, + stopOnTerminationSignals: false, + }); const service = micro(apolloServer.createHandler(options)); const uri = await listen(service); return { diff --git a/packages/apollo-server-plugin-base/src/index.ts b/packages/apollo-server-plugin-base/src/index.ts index a77d6556c16..e6a71b36dcd 100644 --- a/packages/apollo-server-plugin-base/src/index.ts +++ b/packages/apollo-server-plugin-base/src/index.ts @@ -55,13 +55,19 @@ export { export interface ApolloServerPlugin< TContext extends BaseContext = BaseContext -> { - serverWillStart?(service: GraphQLServiceContext): ValueOrPromise; +> extends AnyFunctionMap { + serverWillStart?( + service: GraphQLServiceContext, + ): ValueOrPromise; requestDidStart?( requestContext: GraphQLRequestContext, ): GraphQLRequestListener | void; } +export interface GraphQLServerListener { + serverWillStop?(): ValueOrPromise; +} + export type GraphQLRequestListenerParsingDidEnd = (err?: Error) => void; export type GraphQLRequestListenerValidationDidEnd = ((err?: ReadonlyArray) => void); diff --git a/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts b/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts index 7d4c25305f9..0200d75e3bf 100644 --- a/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts +++ b/packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts @@ -4,6 +4,7 @@ import { GraphQLServiceContext, GraphQLRequestListener, GraphQLRequestContext, + GraphQLServerListener, } from 'apollo-server-plugin-base'; import { /** @@ -97,7 +98,7 @@ for observability purposes, but all operations will be permitted.`, return (): ApolloServerPlugin => ({ async serverWillStart({ engine, - }: GraphQLServiceContext): Promise { + }: GraphQLServiceContext): Promise { logger.debug('Initializing operation registry plugin.'); if (!engine || !engine.serviceID) { @@ -124,6 +125,12 @@ for observability purposes, but all operations will be permitted.`, }); await agent.start(); + + return { + serverWillStop() { + agent.stop(); + }, + }; }, requestDidStart(): GraphQLRequestListener { diff --git a/packages/apollo-server/src/__tests__/index.test.ts b/packages/apollo-server/src/__tests__/index.test.ts index b5d4982fec5..c670a6f3fbb 100644 --- a/packages/apollo-server/src/__tests__/index.test.ts +++ b/packages/apollo-server/src/__tests__/index.test.ts @@ -25,6 +25,35 @@ describe('apollo-server', () => { expect(() => new ApolloServer({ typeDefs, mocks: true })).not.toThrow; }); + it('runs serverWillStart and serverWillStop', async () => { + const fn = jest.fn(); + const beAsync = () => new Promise((res) => res()); + const server = new ApolloServer({ + typeDefs, + resolvers, + plugins: [ + { + async serverWillStart() { + fn('a'); + await beAsync(); + fn('b'); + return { + async serverWillStop() { + fn('c'); + await beAsync(); + fn('d'); + }, + }; + }, + }, + ], + }); + await server.listen(); + expect(fn.mock.calls).toEqual([['a'], ['b']]); + await server.stop(); + expect(fn.mock.calls).toEqual([['a'], ['b'], ['c'], ['d']]); + }); + // These tests are duplicates of ones in apollo-server-integration-testsuite // We don't actually expect Jest to do much here, the purpose of these // tests is to make sure our typings are correct, and to trigger a @@ -102,6 +131,7 @@ describe('apollo-server', () => { server = new ApolloServer({ typeDefs, resolvers, + stopOnTerminationSignals: false, }); const { url } = await server.listen({ port: 0 });