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 using built-in SubscriptionServer error: client gets "Cannot read property 'headers' of undefined" #1537

Closed
tab00 opened this issue Aug 15, 2018 · 22 comments
Labels
🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository.

Comments

@tab00
Copy link

tab00 commented Aug 15, 2018

The server sees client connecting successfully, but the client gets the error, even if nothing is published from server.

Meteor's WebApp is the HTTP server.

  import { ApolloServer } from 'apollo-server-express'
  import { typeDefs } from './schema.js'
  import { resolvers } from './resolvers.js'
  import { DSBooks } from "./DSBooks.js"
  import { getUser } from 'meteor/apollo'

  const server = new ApolloServer({
    typeDefs,
    resolvers,
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req }) => ({ user: await getUser(req.headers.authorization) }),
    uploads: false,
    subscriptions: {
      path: "/subscriptions",
      onConnect: async (connectionParams, webSocket, context) => {
        console.log(`Subscription client connected using Apollo server's built-in SubscriptionServer.`)
      },
      onDisconnect: async (webSocket, context) => {
        console.log(`Subscription client disconnected.`)
      }
     }
  })


  import { WebApp } from 'meteor/webapp'
  server.applyMiddleware({ app: WebApp.connectHandlers }) //path option defaults to /graphql
  WebApp.connectHandlers.use('/graphql', (req, res) => { if (req.method === 'GET') res.end() }) // To prevent server-side exception "Can't set headers after they are sent" because GraphQL Playground (accessible in browser via /graphql) sets headers and WebApp also sets headers

  server.installSubscriptionHandlers(WebApp.httpServer)

If I start a new SubscriptionServer instead, there's no problem, and the client receives subscription data.

  import { ApolloServer } from 'apollo-server-express'
  import { typeDefs } from './schema.js'
  import { resolvers } from './resolvers.js'
  import { DSBooks } from "./DSBooks.js"
  import { getUser } from 'meteor/apollo'

  const server = new ApolloServer({
    typeDefs,
    resolvers,
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req }) => ({ user: await getUser(req.headers.authorization) }),
    uploads: false,
  })


  import { WebApp } from 'meteor/webapp'
  server.applyMiddleware({ app: WebApp.connectHandlers }) //path option defaults to /graphql
  WebApp.connectHandlers.use('/graphql', (req, res) => { if (req.method === 'GET') res.end() }) // To prevent server-side exception "Can't set headers after they are sent" because GraphQL Playground (accessible in browser via /graphql) sets headers and WebApp also sets headers


  import { SubscriptionServer } from 'subscriptions-transport-ws'
  import { execute, subscribe } from 'graphql'
  import { makeExecutableSchema } from 'graphql-tools'

  SubscriptionServer.create(
    {
      schema: makeExecutableSchema({ typeDefs, resolvers }),
      execute,
      subscribe,
      onConnect: async (connectionParams, webSocket) => {
        console.log(`Subscription client connected using new SubscriptionServer.`)
      },
      onDisconnect: async (webSocket, context) => {
        console.log(`Subscription client disconnected.`)
      }
  },
    {
      server: WebApp.httpServer,
      path: "/subscriptions",
    },
  )
@tab00
Copy link
Author

tab00 commented Aug 15, 2018

Code showing ApolloClient creation:

    import ApolloClient from 'apollo-client'
    import { ApolloLink } from 'apollo-link'
    import { MeteorAccountsLink } from 'meteor/apollo'
    import { HttpLink } from 'apollo-link-http'
    import { WebSocketLink } from 'apollo-link-ws'
    import { InMemoryCache } from 'apollo-cache-inmemory'
    import { Meteor } from "meteor/meteor"

    const apolloClient = new ApolloClient({
      link: ApolloLink.split( // split based on operation type
        ({ query }) => {
          import { getMainDefinition } from 'apollo-utilities'
          const { kind, operation } = getMainDefinition(query)
          return kind === 'OperationDefinition' && operation === 'subscription'
        },
        new WebSocketLink({
          uri: `${Meteor.absoluteUrl("/subscriptions").replace("http", "ws")}`,
          options: { reconnect: true }
        }),
        ApolloLink.from([
          new MeteorAccountsLink(),
          new HttpLink({ uri: '/graphql' })
        ]),
      ),
      cache: new InMemoryCache()
    })

@tab00
Copy link
Author

tab00 commented Aug 15, 2018

Both apolloClient.subscribe() and the Subscription React component produce the same error.

@dnalborczyk
Copy link
Contributor

hey @tab00 had the same exception, you could start looking here. I didn't have any headers on the websockets (subscriptions):

context: async ({ req }) => ({ user: await getUser(req.headers.authorization) })
async context(val) {
  console.log(val && val.req)
}

@tab00
Copy link
Author

tab00 commented Sep 18, 2018

This problem still exists. Please fix it.

@ghost
Copy link

ghost commented Sep 19, 2018

Should you use req when using subscriptions? Docs point out that you should use connection to create context from a subscription https://www.apollographql.com/docs/apollo-server/v2/features/subscriptions.html#Context-with-Subscriptions

@tab00
Copy link
Author

tab00 commented Jan 18, 2019

I still get this problem. Has anyone tried to fix this?

@myhendry
Copy link

myhendry commented Feb 4, 2019

@almostprogrammer is correct. I structured my ApolloServer code below and it works. for subscription, you don't use req. u need to use connection
details https://github.com/apollographql/graphql-subscriptions/blob/master/.designs/authorization.md

const server = new ApolloServer({
  typeDefs,
  resolvers,
  context: async ({ req, connection }) => {
    if (connection) {
      return {
        ...connection.context,
        pubsub
      };
    } else {
      const token = req.headers["authorization"] || "";
      return {
        User,
        Tweet,
        Comment,
        pubsub,
        userId: await getUser(token)
      };
    }
  }
});

@tab00
Copy link
Author

tab00 commented Feb 8, 2019

@myhendry, why isn't there subscriptions in the argument list of your ApolloServer instantiation?

@myhendry
Copy link

myhendry commented Feb 8, 2019

@tab00
Copy link
Author

tab00 commented Feb 9, 2019

It looks like returning ...connection.context fixed the problem, so thank you @myhendry for the code.

Though I still needed to add

subscriptions: {
  path: "/subscriptions"

in the ApolloServer arguments object, otherwise I think the built-in subscription server wouldn't start.

So my code is now:

  import { ApolloServer } from 'apollo-server-express'
  import { typeDefs } from './schema.js'
  import { resolvers } from './resolvers.js'
  import { DSBooks } from "./DSBooks.js"
  import { getUser } from 'meteor/apollo'

  const server = new ApolloServer({
    typeDefs,
    resolvers,
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req, connection }) => {
    if (connection) {
      return {
        ...connection.context
      };
    }
    uploads: false,
    subscriptions: {
      path: "/subscriptions",
      onConnect: async (connectionParams, webSocket, context) => {
        console.log(`Subscription client connected using Apollo server's built-in SubscriptionServer.`)
      },
      onDisconnect: async (webSocket, context) => {
        console.log(`Subscription client disconnected.`)
      }
     }
  })

  import { WebApp } from 'meteor/webapp'
  server.applyMiddleware({ app: WebApp.connectHandlers }) //path option defaults to /graphql
  WebApp.connectHandlers.use('/graphql', (req, res) => { if (req.method === 'GET') res.end() }) // To prevent server-side exception "Can't set headers after they are sent" because GraphQL Playground (accessible in browser via /graphql) sets headers and WebApp also sets headers

  server.installSubscriptionHandlers(WebApp.httpServer)

@tab00 tab00 closed this as completed Feb 9, 2019
@myhendry
Copy link

myhendry commented Feb 9, 2019 via email

@jedwards1211
Copy link
Contributor

@helfer the problem here is that SubscriptionServer calls this.context({ connection, payload: message.payload }), hence req is undefined in OP's context function. The only documentation on the website for context shows it being called with the (req, res) from an HTTP request; I can't find any documentation of it being called by SubscriptionServer. Seems like this is either a mistake or the documentation needs to be fixed?

@jedwards1211
Copy link
Contributor

jedwards1211 commented Feb 13, 2019

@helfer to make matters worse, https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/ApolloServer.ts#L480 overwrites the context returned by subscriptions: { onConnect: async (connectionParams: any) => { ... } } that is recommended in the docs on the website.

This is a major issue.
It seems like the developers working on the ApolloServer 2 refactor weren't all on the same page?

@ozanmanav
Copy link

ozanmanav commented Feb 16, 2019

same here its overwrites context with null. I set subscribed user on Connect. When i try in resolvers, its seems undefined.

image

image

@ozanmanav
Copy link

I found a solution Context must be used with connection.context check here.

image

#1796 (comment)

@tab00 tab00 reopened this Feb 17, 2019
@jedwards1211
Copy link
Contributor

@tab00 I documented in #2315 how I think the way both HTTP and Websockets call the context function causes more confusion than it's worth

@jbaxleyiii
Copy link
Contributor

Thanks for reporting this issue originally!

I'm going to close this issue since it hasn't received a lot of traction and could have been resolved already.

If this is still a problem, would someone who's experiencing the problem (or anyone who comes across this issue and is able to assist) mind building a reproduction of the problem in to a runnable CodeSandbox reproduction using the latest version of Apollo Server and sharing the link to that CodeSandbox in this issue?

I'm happy to re-open if this is still occurring and someone can provide a reproduction. Thanks again!

@abernix abernix removed 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged labels Jul 9, 2019
@StefanFeederle
Copy link

StefanFeederle commented Jul 11, 2019

This is still an issue for most people who setup auth with subscriptions.
It's possible to get it working but really not as easy as it should be.
Please see #2315 for proposed api change. Other related issue: #1597

This is a running sandbox showing the problem: https://codesandbox.io/s/apollo-server-qthfh?fontsize=14

@jedwards1211
Copy link
Contributor

@StefanFeederle that seems like the wrong issue number, mind checking it?

@StefanFeederle
Copy link

@jedwards1211 Thanks, fixed it

@abernix abernix reopened this Jul 28, 2019
@abernix abernix added 🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository. labels Jul 28, 2019
@Landoooooo
Copy link

Is this dead? I'm having issues with receiving the connection param from context. I'm using websockets for auth If that's relevent.

@glasser
Copy link
Member

glasser commented Mar 4, 2022

Apollo Server no longer has a built-in SubscriptionServer.

@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
None yet
Development

No branches or pull requests