Skip to content

Commit

Permalink
Triggers for newly introduced serverWillStart lifecycle hooks.
Browse files Browse the repository at this point in the history
This commit follows-up on #1795, which introduced the new request pipeline, and
implements the triggers for its new life-cycle hooks within the various
integration packages.  Previously, the only implementation was within the
`apollo-server` package and didn't get triggered for those invoking the
`applyMiddleware` method on integrations (e.g. `apollo-server-express`,
`...-hapi` and `...-koa`).

While in an ideal world, ALL existing `applyMiddleware` functions would be
marked as `async` functions and allow us to `await ApolloServer#willStart`,
in practice the only `applyMiddleware` which is currently `async` is the one
within the Hapi implementation.  Therefore, we'll instead kick off the
`willStart` lifecycle hook as soon as `applyMiddleware` is started, return
as quickly as we have before and then (essentially) yield the completion of
Apollo Server's `willStart` prior to serving the first request — thus
ensuring the completion of server-startup activities.

Similarly, we'll do the same for `createHandler` methods on integrations
which don't utilize Node.js server frameworks but don't have `async`
handlers (e.g. AWS, Lambda, etc.).
  • Loading branch information
abernix committed Oct 26, 2018
1 parent 50a76d1 commit aae212f
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 9 deletions.
20 changes: 16 additions & 4 deletions packages/apollo-server-cloud-function/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ 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<string, any>;

if (cors) {
Expand Down Expand Up @@ -132,10 +137,17 @@ export class ApolloServer extends ApolloServerBase {

res.set(corsHeaders);

graphqlCloudFunction(this.createGraphQLServerOptions.bind(this))(
req,
res,
);
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);
};
}
}
1 change: 1 addition & 0 deletions packages/apollo-server-cloudflare/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class ApolloServer extends ApolloServerBase {
}

public async listen() {
await this.willStart();
const graphql = this.createGraphQLServerOptions.bind(this);
addEventListener('fetch', (event: FetchEvent) => {
event.respondWith(graphqlCloudflare(graphql)(event.request));
Expand Down
20 changes: 20 additions & 0 deletions packages/apollo-server-express/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ export class ApolloServer extends ApolloServerBase {
return true;
}

// TODO: While `express` is not Promise-aware, this should become `async` in
// a major release in order to align the API with other integrations (e.g.
// Hapi) which must be `async`.
public applyMiddleware({
app,
path,
Expand All @@ -94,6 +97,23 @@ export class ApolloServer extends ApolloServerBase {
}: ServerRegistration) {
if (!path) path = '/graphql';

// 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();
app.use(path, (_req, _res, next) => {
promiseWillStart.then(() => next()).catch(next);
});

if (!disableHealthCheck) {
// uses same path as engine proxy, but is generally useful.
app.use('/.well-known/apollo/server-health', (req, res) => {
Expand Down
2 changes: 2 additions & 0 deletions packages/apollo-server-hapi/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ export class ApolloServer extends ApolloServerBase {
disableHealthCheck,
onHealthCheck,
}: ServerRegistration) {
await this.willStart();

if (!path) path = '/graphql';

await app.ext({
Expand Down
24 changes: 24 additions & 0 deletions packages/apollo-server-koa/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export class ApolloServer extends ApolloServerBase {
return true;
}

// TODO: While Koa is Promise-aware, this API hasn't been historically, even
// though other integration's (e.g. Hapi) implementations of this method
// are `async`. Therefore, this should become `async` in a major release in
// order to align the API with other integrations.
public applyMiddleware({
app,
path,
Expand All @@ -84,6 +88,26 @@ export class ApolloServer extends ApolloServerBase {
}: ServerRegistration) {
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();
app.use(
middlewareFromPath(path, async (_ctx: Koa.Context, next: Function) => {
await promiseWillStart;
return next();
}),
);

if (!disableHealthCheck) {
// uses same path as engine proxy, but is generally useful.
app.use(
Expand Down
21 changes: 16 additions & 5 deletions packages/apollo-server-lambda/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ 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: lambda.APIGatewayProxyResult['headers'] = {};

if (cors) {
Expand Down Expand Up @@ -156,11 +161,17 @@ export class ApolloServer extends ApolloServerBase {
);
};

graphqlLambda(this.createGraphQLServerOptions.bind(this))(
event,
context,
callbackFilter,
);
graphqlLambda(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);
})(event, context, callbackFilter);
};
}
}
6 changes: 6 additions & 0 deletions packages/apollo-server-micro/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,15 @@ 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();

return async (req, res) => {
this.graphqlPath = path || '/graphql';

await promiseWillStart;

await this.handleFileUploads(req);

(await this.handleHealthCheck({
Expand Down

0 comments on commit aae212f

Please sign in to comment.