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

doc: Subscription example doesn't include connection param #30

Open
raymclee opened this issue Apr 19, 2020 · 5 comments
Open

doc: Subscription example doesn't include connection param #30

raymclee opened this issue Apr 19, 2020 · 5 comments
Labels
accepting help Issues open to PRs or contributions by others documentation An issue or PR primarily about updating documentation good first issue Good for newcomers

Comments

@raymclee
Copy link

raymclee commented Apr 19, 2020

Hi, query and mutation work perfectly but not working with subscription.

When i try to call ctx.getUser(), ctx.isAuthenticated() or ctx.isUnauthenticated() will get the following errors:
Cannot read property 'user' of undefined
Cannot read property 'isAuthenticated' of undefined
Cannot read property 'isUnauthenticated' of undefined

Also, there is a type errors on createOnConnected() which is

Type '(connectionParams: Object, webSocket: WebSocket<Request<ParamsDictionary, any, any, Query>>) => Promise' is not assignable to type '(connectionParams: Object, websocket: WebSocket, context: ConnectionContext) => any'.
Types of parameters 'webSocket' and 'websocket' are incompatible.
Property 'upgradeReq' is missing in type 'WebSocket' but required in type 'WebSocket<Request<ParamsDictionary, any, any, Query>>'.ts(2322)
types.d.ts(33, 5): 'upgradeReq' is declared here.

here is the server

const server = new ApolloServer({
  schema,
  context: ({ req, res }) => buildContext({ req, res, prisma, pubsub }),
  playground: {
    settings: {
      "request.credentials": "include",
    },
  },
  subscriptions: {
    // @ts-ignore
    onConnect: createOnConnect([
      sessionMiddleware,
      passportMiddleware,
      passportSessionMiddleware,
    ]),  
   },
});
@jkettmann
Copy link
Collaborator

Thanks for this issue. Can you reproduce this issue in an example repo or codesandbox? That would be very helpful

@dkluhzeb
Copy link

I have the same issue.

upgradeReq was removed from ws in 3.0: https://github.com/websockets/ws/releases/tag/3.0.0

my try was to set the property manually (websockets/ws#1099 (comment)):

    subscriptions: {
      keepAlive: 10000,
      onConnect: (connectionParams, webSocket, connectionContext) => {
        webSocket.upgradeReq = connectionContext.request

        const onConnect = createOnConnect([
          expressSession,
          passportInstance,
          passportSession
        ])

        return onConnect(connectionParams, webSocket)
      }

But saldy this doesnt worked. The wrapped onConnect function returns correctly the ReturnOnConnect object. { req: <request (incoming message) with middleware>} but the request is not setted in the context.req.

Is there a known solution or workaround?

@yalnjooj
Copy link

yalnjooj commented Jan 2, 2021

I have the same issue, Is there a known solution or workaround?

@jkettmann
Copy link
Collaborator

@dkluhzeb thanks for the details. Very helpful. With that information I can soon create a PR.

I'm curious though: What server packages (e.g. apollo server or apollo server express) and what version are you using? My test project still works fine. I also don't see why your workaround shouldn't work.

Did you setup the GraphQL context with the buildContext function like this?

context: ({ req, res, connection }) => buildContext({ req, res, connection }),

@jkettmann jkettmann added the bug Something isn't working label Jan 6, 2021
@jpbidal
Copy link

jpbidal commented Mar 31, 2021

Hello! I had the same situation like @raymclee
@jkettmann My buildContext had no connection param, that was the problem. I tried it after read your comment and my credentials were loaded successfully.

@raymclee the change must be:

const server = new ApolloServer({
  schema,
  // see new connection param
  context: ({ req, res, connection }) => buildContext({ req, res, connection, prisma, pubsub }),
  playground: {
    settings: {
      "request.credentials": "include",
    },
  },
  subscriptions: {
    // @ts-ignore
    onConnect: createOnConnect([
      sessionMiddleware,
      passportMiddleware,
      passportSessionMiddleware,
    ]),  
   },
});

P.S.=Thank you Johannes for this library, and congrats for your great job. Maybe you can add the connection param to the subscription section's tutorial

@ericbf ericbf added accepting help Issues open to PRs or contributions by others documentation An issue or PR primarily about updating documentation good first issue Good for newcomers and removed bug Something isn't working labels Dec 21, 2022
@ericbf ericbf changed the title Not working with subscription doc: Subscription example doesn't include connection param Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting help Issues open to PRs or contributions by others documentation An issue or PR primarily about updating documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants