prisma-labs / graphql-framework-experiment

Code-First Type-Safe GraphQL Framework
https://nexusjs.org
MIT License
675 stars 66 forks source link

Built-in server support for subscriptions #897

Closed jasonkuhrt closed 4 years ago

jasonkuhrt commented 4 years ago

Perceived Problem

Ideas / Proposed Solution(s)

danielmahon commented 4 years ago

Just throwing another option out there for reference, though I'm sure you'd rather build it directly into the framework: https://github.com/apollographql/subscriptions-transport-ws works well and works "now" with the current nexus if you pass it the HTTP server and schema. Uses https://www.npmjs.com/package/ws under the hood. See my comment in #447 for how I'm using it to support subscriptions in the current nexus build. (the plugin nexus-plugin-subscriptions just wraps subscriptions-transport-ws)

jasonkuhrt commented 4 years ago

Thanks @danielmahon adding that possibility into the OP 👍

jasonkuhrt commented 4 years ago

Saw this via colleague should be something that is easy to build with Nexus down the line https://databaselog.com/ (in part if/when Prisma has real-time support too)

jasonkuhrt commented 4 years ago

Maybe relevant library for us https://github.com/apollographql/graphql-subscriptions. But looks inactive.

danielmahon commented 4 years ago

@jasonkuhrt It's been working in my setup with nexus/prisma so far, but yeah I'm unsure of its future. Although the latest stable Apollo server is still using it... 🤔 , and I don't see them dropping subscription support anytime soon. I haven't dug very far into its code yet but the main issue with its current state seems to be a memory leak with the withFilter function (https://github.com/apollographql/graphql-subscriptions/pull/209). I've tested the fork too and had no issues using it combined with Google's PubSub and over 100k+ updates/day.

leungandrew commented 4 years ago

@danielmahon with #1050 you don't need your fork of Nexus anymore and can go back to using the latest. I was able to get subscriptions working with your plugin by doing the following:

use(
  subscriptions({
    ws: { server: server.raw.http, path: '/graphql' }, // use server.raw.http here
    keepAlive: 10 * 1000,
    onConnect: (payload: Record<string, any>) => {
      log.info('client connected')
      // return ctx
      return { pubsub, db, log }
    },
    onDisconnect: () => {
      log.info('client disconnected')
    },
  }),
)

Cheers and thanks for the plugin!

Sytten commented 4 years ago

The subscriptions-transport-ws package has effectively not been updated for two years and has a lot of bugs in it (still marked work in progress even though everybody uses it in prod). One thing that is very annoying is it generally not well integrated in the apollo server for the lifecycle of a request, error management, context creation, etc. It feels like subscriptions are second-class citizens instead of being as nice to use as queries and mutations. I would really like to see a new implementation of the transport layer. As for the graphql-subscriptions package, it is worth keeping to access all the plugins the community created.

jasonkuhrt commented 4 years ago

Thanks for the input @Sytten. Indeed everything you're saying lines up with how I suspected things were. There's also issues with context management deeper down the lib stack https://github.com/graphql/graphql-js/issues/894.

Do you have any lib suggestions we could add to our list of possible tools to use above?

Sytten commented 4 years ago

Interesting, I don't think the apollo implementation suffers from the issue mentioned in GraphQLjs. They do create a new context on each message as far as i can tell.

I think it would be nice to have a generic transport interface that could be changed by the user. Websocket is ok for most cases, but one might want to use Pusher in a serverless case or another third party.

paniavula commented 4 years ago

First class support for subscription transport interface would be really helpful

jasonkuhrt commented 4 years ago

This will be closed soon as we're building on top of Apollo Server now which provides subscriptions support.

Sytten commented 4 years ago

Seems fair, but please keep an eye on their implementation since it is pretty bad currently.

jasonkuhrt commented 4 years ago

@Sytten we're open to something better in the future and in serverless will need a whole other technical solution too (e.g. seamless AWS API Gateway Websockets, Pusher, etc. integration).

brielov commented 4 years ago

I see you've added subscriptionType on 0.26 and you're already using apollo-server-express so I went ahead and tried it and graphql playground throws:

Could not connect to websocket endpoint ws://localhost:4000/graphql. Please check if the endpoint url is correct.

Am I missing some configuration or it isn't ready for testing yet?

jasonkuhrt commented 4 years ago

@brielov isn't ready yet. When this issue is closed, it will be though.

brielov commented 4 years ago

@jasonkuhrt Sorry, my bad. Thank you!

brielov commented 4 years ago

Any news on this?

Albert-Gao commented 4 years ago

Could there be two modes for the subscription?

  1. Serverful mode
  2. AWS Serverless mode integrates with WebSocket from API Gateway
jasonkuhrt commented 4 years ago

@Albert-Gao yes that's my vision. Add in others too, like pusher. Be pluggable etc.

I will try to push something in the near future that puts the Apollo Server subscriptions server to use. PRs welcome. My plan is that if you use Subscription type in your schema it will be automatically enabled, otherwise automatically disabled, and then manual overrides will be possible too.

Albert-Gao commented 4 years ago

Looks great, seems the code is already merged!

Just got a great idea, we can use the local apollo for testing subscription, and using AWS API gateway(or other services) for actual deployment... Not a 1-to-1 matching env for sure, but solves the painpoint of serverless dev in an elegant way, since if the abstraction from nexus is stable, that is the part we should not worry about.

What a beautiful future.