Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscriptions onConnect return value is not appended to context #1597

Closed
samalexander opened this issue Aug 31, 2018 · 9 comments
Closed

Subscriptions onConnect return value is not appended to context #1597

samalexander opened this issue Aug 31, 2018 · 9 comments
Labels
🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository.

Comments

@samalexander
Copy link

According to the docs for subscriptions in apollo-server-express 2.0, the return value of onConnect is appended to the context. However, in practice the return value of onConnect is ignored and only the context returned by the context method of ApolloServer config is available in resolvers.

For example, with this code:

const server = new ApolloServer({
    // These will be defined for both new or existing servers
    typeDefs,
    resolvers,
    context: async ({ req, connection }) => {
        if (connection) {
            // check connection for metadata
            return {
                foo: 'bar'
            };
        } else {
            // check from req
            const token = req.headers.authorization || "";

            return { token };
        }
    },
    subscriptions: {
        onConnect: (connectionParams, webSocket) => {
            console.log('Websocket CONNECTED');
            return {
                hello: 'world'
            }
        },
        onDisconnect: () => console.log('Websocket CONNECTED'),
    }
});

You would expect the context for websockets to be:
{foo: 'bar', hello: 'world'}
But in practice the context will be:
{foo: 'bar'}

I have created a basic reproduction of the issue here.

@ghost ghost added ✋ blocking Prevents production or dev due to perf, bug, build error, etc.. 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository. labels Aug 31, 2018
@samalexander
Copy link
Author

Issue is still present in v2.1.0

@sabith-th
Copy link

sabith-th commented Sep 21, 2018

I was able to get hello object in the context if I manually add it to the context via connection.context. Like:

context: async ({ req, connection }) => {
        if (connection) {
            // check connection for metadata
            return {
                foo: 'bar',
                hello: connection.context.hello,  
            };
        } else {
            // check from req
            const token = req.headers.authorization || "";

            return { token };
        }
    },
    subscriptions: {
        onConnect: (connectionParams, webSocket) => {
            console.log('Websocket CONNECTED');
            return {
                hello: 'world'
            }
        },
        onDisconnect: () => console.log('Websocket CONNECTED'),
    }

Not sure if this is the correct approach though.

@samalexander
Copy link
Author

samalexander commented Sep 28, 2018

Nice one @sabith-th. With that being the case, we can simply use the following as a workaround:

return Object.assign(
        {
          foo: "bar"
        },
        connection.context
      );

@arist0tl3
Copy link

Just curious if anyone would find this approach helpful or have feedback =)

Right now I pass the token in two places:

First in the middleware, so that the token is always fresh on each call and allows for using withFilter with a user context that we know is current. NOTE: you need to have your lookup function inside of the applyMiddleware function, as it will be called on each request. If you declare it at the top of the file, it will be stale.

Second, in the connection params so that onConnect can see it and write it to context so that later onDisconnect can also see it, so that we can rely on those calls to set user online status.

// Client.js

import get from 'lodash/get';
import { WebSocketLink } from 'apollo-link-ws';

const wsLink = new WebSocketLink({
  uri: 'ws://localhost:4000/graphql',
  options: {
    // Pass the token as a connection param so we can set the online status
    // on the onConnect callback in subscriptions
    connectionParams: {
      authToken: `Bearer ${get(JSON.parse(localStorage.getItem('user')), 'data.token')}`,
    },
    reconnect: true,
  },
});

// Pass the authToken as an option on each socket call so that the user
// data is never stale
const subscriptionMiddleware = {
  applyMiddleware: async (options, next) => {
    // eslint-disable-next-line no-param-reassign
    options.authToken = `Bearer ${get(JSON.parse(localStorage.getItem('user')), 'data.token')}`;
    next();
  },
};

// add the middleware to the web socket link via the Subscription Transport client
wsLink.subscriptionClient.use([subscriptionMiddleware]);

export default wsLink;
// Server.js

const server = new ApolloServer({
  context: async ({ req, payload }) => {
    // Allow getting the auth from req or ws payload
    const authToken = get(req, 'headers.authorization') || get(payload, 'authToken');
    const user = await AuthServices.getUser(authToken);

    if (!user) {
      return { ...req, pubsub, models };
    }

    return {
      ...req,
      pubsub,
      user,
      models,
    };
  },
  formatError,
  playground: {
    endpoint: '/playground',
  },
  schema,
  subscriptions: {
    onConnect: async ({ authToken }) => {
      if (authToken) {
        const user = await AuthServices.getUser(authToken);
        if (user) {
          await user.setOnline();
          return {
            authToken,
          };
        }
      }
      return {};
    },
    onDisconnect: (params, socket) => {
      socket.initPromise.then(async ({ authToken }) => {
        if (authToken) {
          const user = await AuthServices.getUser(authToken);
          if (user) {
            await user.setOffline();
          }
        }
      });
    },
    path: '/graphql',
  },
});

@jedwards1211
Copy link
Contributor

jedwards1211 commented Feb 13, 2019

This is caused by https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/ApolloServer.ts#L480 overwriting the context returned by onConnect.

jedwards1211 added a commit to jedwards1211/apollo-server that referenced this issue Feb 13, 2019
jedwards1211 added a commit to jedwards1211/apollo-server that referenced this issue Feb 13, 2019
@jedwards1211
Copy link
Contributor

jedwards1211 commented Feb 13, 2019

@samalexander the only proper workaround I can see for right now is to fork the ApolloServer.installSubscriptionHandlers function and either make it not call this.context or make it merge the objects from onConnect and this.context.

@jedwards1211
Copy link
Contributor

jedwards1211 commented Feb 13, 2019

@sabith-th actually it would be even better to do this:

        if (connection) {
            return {
                ...connection.context,
                foo: 'bar',
            };

@Ponjimon
Copy link

Ponjimon commented Sep 2, 2019

What's the current status on this? This issue is open for over a year now and it's still not working

@glasser
Copy link
Member

glasser commented Mar 4, 2022

Apollo Server no longer has a superficial integration with a subscriptions server. (A future version hopefully will have a deeper subscriptions integration that avoids this issue.)

@glasser glasser closed this as completed Mar 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository.
Projects
No open projects
Design: Subscriptions
Awaiting triage
Development

No branches or pull requests

7 participants