Apollo-server: usage reporting doesn't work with old graphqlExpress middleware in apollo-server v2.18

[glasser]

(This issue is linked from an error message, so the description has been updated to be relevant.)

In some cases, the Apollo usage reporting plugin (which sends information on how your server is being used to Apollo's servers to power Apollo Studio) can find itself in a state where it's serving a GraphQL request without ever having been properly started up.

The main reason this can happen is if you don't use supported ApolloServer 2.x APIs to connect your ApolloServer to your web server. For example, let's say you've written this code:

const { ApolloServer } = require('apollo-server-express');
const { graphqlExpress } = require('apollo-server-express/dist/expressApollo');

const server = new ApolloServer(options);
app.use('/path',  async (req, res, next) => {
  const serverOptions = await server.createGraphQLServerOptions(req, res)
  return graphqlExpress(serverOptions)(req, res, next)
})

Note that this is importing the function graphqlExpress from the unsupported dist subdirectory of apollo-server-express; graphqlExpress is a function that was part of the apollo-server 1.x API (before the ApolloServer class existed) but is not part of the 2.x API.

When you use this internal function, the full Apollo Server lifecycle doesn't take place, and so usage reporting doesn't work properly. You should instead use use the applyMiddleware or getMiddleware methods on ApolloServer, which properly start your server's plugin lifecycle. Note that by default applyMiddleware adds cors and body-parser middlewares, so to maintain the same behavior, you can disable them:

const { ApolloServer } = require('apollo-server-express');

const server = new ApolloServer(options);
server.applyMiddleware({ app, path: '/path', cors: false, bodyParserConfig: false });

[glasser]

While I'm here I note that legacyOptions.ts isn't copying logger over.

[glasser]

This error happens if you try to use usage reporting in AS v2.18 and you’re using the old plain middleware API (just calling graphqlExpress yourself) instead of using ApolloServer.applyMiddleware or getMiddleware.

For example, the reporting user had this code returning an express middleware:

const apolloServer = (options) => {
  const server = new ApolloServer(options)

  return async (req, res, next) => {
    const serverOptions = await server.createGraphQLServerOptions(req, res)
    return graphqlExpress(serverOptions)(req, res, next)
  }
}

This doesn't call serverWillStart plugins!

A fix for this user was to change to:

const apolloServer = (options) => {
  const server = new ApolloServer(options);
  return server.getMiddleware({ cors: false, bodyParserConfig: false });
}

But we should make sure the experience is better for folks using these old modules.

Possible solutions include one or more of:

• Throwing a better error if requestDidStart gets called in the usage reporting plugin before serverWillStart
• Actually calling ApolloServer.willStart from createGraphQLServerOptions? I think it's nice that when you're using an HTTP server framework it starts running willStart nice and early and blocks everything (including loading playground etc) on it (so that errors loading it show up instead of playground, say)... but maybe if createGraphQLServerOptions detects that willStart hasn't been called yet it should still call it? This might be tied in to #4435 which is also about willStart semantics.

Unclear if it's OK for us to just decide that you have to use a slightly more modern API to do usage reporting.

[glasser]

Heh, the old middlewares aren't exported. So I think I'll just improve the error to link to here.

[mmeylan]

@glasser How does that apply to apollo-server-testing and createTestClient ? Currently experiencing this issue with jest on CI with the following code:

import {ApolloServer} from 'apollo-server-express'
const server =  new ApolloServer({
    schema,
    plugins: [],
    engine: {
      reportSchema: false,
      apiKey: apolloApiKey, // apolloApiKey is undefined in unit tests
    },
})

// ...

import {createTestClient} from 'apollo-server-testing'
const {query} = createTestClient(server)
    const res = await query({
      query: gql`...`
    })
})

Edit

I found out that my issue in CI only was due to the fact that my CI exposes a variable APOLLO_KEY, designed for deployments and not tests. ApolloServer in unit tests was still picking it up and automatically setting up a reporting plugin which was failing my tests.

I must say that this behavior is dangerous and took me a while to debug as I expected that setting config.engine.apiKey to undefined would not be overriden by APOLLO_KEY.

Leaving this here as it may help somebody else.

[glasser]

Hi, I see your concern. It would make sense if setting apollo.key to null or maybe undefined would override the environment variable. (apollo.key is the new place to put engine.apiKey.) I don't believe this is a v2.18 regression, though; I don't believe we've ever supported a way to override $APOLLO_KEY with nothing.

I think having apollo: {key: undefined} be different from apollo: {} would be surprising since in JS undefined values in objects are generally ignored (eg, by JSON.stringify), but it would make sense if apollo: {key: null} did what you wanted. I'd review a PR that did that (with full docs update etc).

The main impact of APOLLO_KEY is enabling usage reporting (schema reporting is not on by default); as of v2.18 you can explicitly disable the plugin in your tests with https://www.apollographql.com/docs/apollo-server/api/plugin/usage-reporting/#disabling-the-plugin

[mmeylan]

Thank you very much for the detailed response @glasser. I think it's great that this lives in this thread as it is linked in Apollo's error people will experience if they make the same mistake as me. Disabling the plugin is very useful indeed.

For my very specific issue with APOLLO_KEY set in CI (for deployments), I chose to simply remove APOLLO_KEY in my test setup and avoid having if(isTest) ... else ... in my server setup code.

// jest.setup.ts
delete process.env.APOLLO_KEY