andyrichardson / subscriptionless

GraphQL subscriptions (and more) on serverless infrastructure
MIT License
93 stars 3 forks source link

Subscription Input #14

Closed reconbot closed 3 years ago

reconbot commented 3 years ago

I'm trying to reject a subscription based upon the arguments to the subscription.

My code looks like this (using nexusjs)

export const watchChannel = extendType({
  type: 'Subscription',
  definition(t) {
    t.field('watchChannel', {
      type: Channel,
      args: {
        id: nonNull(idArg()),
      },
      async subscribe(root, args, context){
        console.log({ root, args, context })
        const { id } = args
        const { db } = context
        if (!await db.channel.get(id)) {
          throw new Error('Unknown channel')
        }
        return subscribe('NumbersStationUpdate') 
        // you could call it - doesn't matter 
        // return subscribe('NumbersStationUpdate')(root, args, context)
      },
      resolve({ payload }) {
        return payload.num
      },
    })
  },
})

I'd like to propose an alternative. If the function doesn't have a .definitions property, execute it (and resolve it) and see if the result has a .definitions property, then create the subscription objects.

Also we can provide a finished Async iterator from subscribe() so the types check out and it just wont do anything, but then we could allow a typed response from subscribe

Thoughts?

andyrichardson commented 3 years ago

Thanks for the report!

Errors on subscribe

If I'm getting this right, when a subscription is initially established and an error is thrown during the "subscribe" function execution, you want to be able to send an error rather than closing the connection outright.

Whether we do this would be entirely dependent on the protocol. Looking at the docs

Operation execution error(s) triggered by the Next message happening before the actual execution, usually due to validation errors.

This leads me to think that an error message might not be appropriate as it seems to be intended for errors happening on the resolve resolver call. The best way to work this out is probably going to be to reproduce this with a graphql-ws backend and see how it handles it.

Return types don't match traditional GraphQL

The return types are accurate - if the output of subscriptionless was used in a traditional GraphQL setup, it would fail at runtime (hence the type errors ahead of time).

I think the most type-safe route would be to use declaration merging so that nexus both expects, and returns the correct type definitions.

reconbot commented 3 years ago

Look like I can conditionally return a subscriptionless subscribe() call. The subscribe method does execute, I was incorrect.

Our behavior is the same as graphql-ws, just close the connection.

From testing graphql-ws

{ message: '{"type":"connection_init","payload":{}}' }
{ send: '{"type":"connection_ack"}' }
{ message: '{"id":"39abc299-e678-49a1-9069-be6a015b0f04","type":"subscribe","payload":{"query":"query hello {\\n  error\\n}\\n\\n\\nsubscription greeting {\\n  greetings\\n}","variables":{},"operationName":"greeting"}}' }
Connection closed
andyrichardson commented 3 years ago

Thanks for investigating @reconbot