Calling context function with WebSocket connection parameters creates confusion; either docs or API need improvement

[jedwards1211]

I'll be happy to make a PR to clean up the API or improve the documentation after discussing the best solution to this with folks.

Looking at the code example in https://www.apollographql.com/docs/apollo-server/v2/features/authentication.html#context, it looks like context will always be called with the req and res from the HTTP middleware:

const server = new ApolloServer({
 typeDefs,
 resolvers,
 context: ({ req }) => {
   // get the user token from the headers
   const token = req.headers.authorization || '';
   ...
 },
});

However, this is not so. As soon as one starts using subscriptions, they get a rude awakening (Issue #1537): now context is called with the WebSocket connection, and they get the error Cannot read property 'headers' of undefined when they try to read req.headers.

It was only after thoroughly debugging this issue and seeing what was happening under the hood that I managed to find the docs that actually show a grand unified context function: https://www.apollographql.com/docs/apollo-server/v2/features/subscriptions.html#Context-with-Subscriptions

const server = new ApolloServer({
  schema,
  context: async ({ req, connection }) => {
    if (connection) {
      // check connection for metadata
      return connection.context;
    } else {
      // check from req
      const token = req.headers.authorization || "";

      return { token };
    }
  },
});

But since it's called in two completely different ways, and the code paths for the two are entirely separated (here at least), I don't think it was a very good candidate for unification.

Adding to the confusion is the fact that subscription params have to be accessed from a separate subscriptions.onConnect function which is completely missing from the above example. Clearly I'm not the only one who was confused by the interplay between these two functions; others were also surprised that the context returned from onConnect can get blown away if not all of its properties are returned from the context function (#1597).

The example of onConnect I found in the docs makes it look like the single source of truth for subscription context. Notice how it is not just extracting values from connectionParams; it's also computing derived context props, which it seems like the context function is supposed to be responsible for:

const server = new ApolloServer({
  typeDefs,
  resolvers,
  subscriptions: {
    onConnect: (connectionParams, webSocket) => {
      if (connectionParams.authToken) {
        return validateToken(connectionParams.authToken)
          .then(findUser(connectionParams.authToken))
          .then(user => {
            return {
              currentUser: user,
            };
          });
      }

      throw new Error('Missing auth token!');
    },
  },
});

It seems like the intended use of the API is actually something like:

const server = new ApolloServer({
  schema,
  context: async ({ req, connection }) => {
    let authToken;
    if (connection) {
      ({authToken}) = connection.context;
    } else {
      const match = /^Bearer (.*)$/.exec(req.headers.authorization);
      if (match) authToken = match[1]
    }

    return {
      authToken,
      userId: await getUserId(authToken),
    };
  },
  subscriptions: {
    onConnect: async (connectionParams: any) => {
      const { authToken } = connectionParams;
      return { authToken };
    },
  },
});

But I would argue that we should either

• Get rid of subscriptions.onConnect and call context with connectionParams directly
• or not call context for subscriptions at all, and rely solely on subscriptions.onConnect to get the context
• or document the context function, subscriptions.onConnect, and how they relate a lot more thoroughly, for example:
  • warning in the HTTP auth docs that req may be undefined in the case that the function is called for a subscriptions WebSocket
  • warning in the onConnect docs that its return value is passed to the context function as connection.context and must be returned intact to avoid losing any properties
  • adding an onConnect function to the example in the "Context with Subscriptions" docs

cc @samalexander, @arist0tl3, @sabith-th, @jbaxleyiii, @evans

Personally, I think it would be best to have one function in charge of extracting params from the request, one function in charge of extracting params from the websocket connection, and one function that takes params and returns the context. Here is how that would look:

return new ApolloServer({
  schema,
  formatError,
  context: async ({ authToken }: Params): Promise<GraphQLContext> => {
    let userId = null
    if (authToken) {
      ({ user: { id: userId } } = await loginWithToken(null, authToken))
    }
    return {
      userId,
      authToken,
      sequelize,
    }
  }
  getParamsFromRequest: ({ req }: { req: $Request }): Params => {
    const auth = req.get('authorization')
    const match = /^Bearer (.*)$/i.exec(auth || '')
    const authToken = match ? match[1] : null
    return { authToken }
  },
  subscriptions: {
    getParamsFromConnection: ({ authToken }: ConnectionParams): Params => {
      return { authToken }
    },
  },
})

[cheapsteak]

Thanks so much for the incredibly thorough description!
I'll be putting some time into investigating this, your guide here is a tremendous help
It looks like I still need to read through quite a bit of code to catch up to the level of understanding required for helping with this problem, I wanted to drop a comment off first to let you know that it's not being ignored

From a purely API-design POV (technical limitations aside), my initial instinct is to avoid adding to the surface area of the ApolloServer constructor's API if possible. Would it make sense to think of context as a sort of redux-like reducer, where each payload could be a "type" of either "subscription" or "http request"? That way the handlers for each could still be separate functions, but they wouldn't need to be a part of the constructor API (this would be along the lines of your suggestion to get rid of subscriptions.onConnect)

E.g.

const server = new ApolloServer({
  schema,
  context: ({ req, connection }) => {
    if (connection) {
      return getContextFromSubscription(connection);
    }
    return getContextFromHttpRequest(req);
  },
});

where getContextFromHttpRequest could be a user defined function (likely with not such a lengthy name), like

const getContextFromHttpRequest = ({ req }: { req: $Request }): Params => {
    const auth = req.get('authorization')
    const match = /^Bearer (.*)$/i.exec(auth || '')
    const authToken = match ? match[1] : null
    return {
      authToken,
      user: await getUserByToken(null, authToken)),
    }
  }

What do you think?

[priyankashah2017]

const server = new ApolloServer({
   schema,
  context: async ({ req, connection }) => {
      let authToken;
     if (connection) {
          ({authToken}) = connection.context;
     } else {
     const match = /^Bearer (.*)$/.exec(req.headers.authorization);
     if (match) authToken = match[1]
   }
  return {
   authToken,
   userId: await getUserId(authToken),
   };
},
subscriptions: {
  onConnect: async (connectionParams: any) => {
     const { authToken } = connectionParams;
     return { authToken };
   }
  }
});

Its working for me.
Thanks a lot

[jedwards1211]

@cheapsteak yeah what you propose is probably a lot easier to understand, I don't know what I was thinking with what I proposed.

To summarize, context can already be called with two argument types: {req} and {connection}. But as you propose it would be better if ApolloServer called context with {connectionParams} instead of {connection}. Because it's not immediately obvious that connection is what subscriptions.onConnect returned.

FWIW taking multiple argument types isn't the defining characteristic of a reducer...the name "reducer" comes from the fact that actions.reduce(reducer, state) will give you the state that results from applying a bunch of actions to an initial state. context doesn't work with array.reduce so it's not a reducer.

[aadibajpai]

came here trying to figure out how to get the subscription context passed and I found a lot of users doing something like context: async ({ req, connection }) but I couldn't find supporting documentation. It appears https://web.archive.org/web/20180723162856/https://www.apollographql.com/docs/apollo-server/v2/features/subscriptions.html doesn't exist anymore and that piece of info didn't make it to the new subscription docs.

[glasser]

Apollo Server 3 no longer has a superficial built-in subscriptions integration, so I'm closing this issue.

If you choose to integration AS3 with subscriptions-transport-ws, I hope that https://www.apollographql.com/docs/apollo-server/data/subscriptions/#operation-context helps a bit with understanding how subscriptions context works.