andyrichardson / subscriptionless

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

Mutation and Query Support #18

Closed reconbot closed 3 years ago

reconbot commented 3 years ago

I've been using the Altair graphql client and it's got a behavior. If a websocket url is set it uses it for all queries, mutations and subscriptions. I looked into the graphql-ws client and it's an option to turn on and off. However, queries and mutations throw an error as they don't have a subscribe method.

TypeError: field.subscribe is not a function
andyrichardson commented 3 years ago

If the graphql-ws protocol supports queries and mutations, I'll see what I can do to add support :+1:

reconbot commented 3 years ago

It sure does. I got a working prototype on my fork, which is correct enough to make the graphql-ws client happy.

https://github.com/reconbot/reconbot-subscriptionless/blob/master/src/messages/subscribe.ts#L65-L95

      if (execContext.operation.operation !== 'subscription') {
        // todo reuse existing execContext because it's duplicating a lot of work
        const result = await execute(
          c.schema,
          parse(message.payload.query),
          undefined,
          await constructContext(c)({ connectionParams }), // look into making this consistent across every call to subscribe and resolve
          message.payload.variables,
          message.payload.operationName,
          undefined
        );

        await sendMessage(c)({
          ...event.requestContext,
          message: {
            type: MessageType.Next,
            id: message.id,
            payload: result,
          },
        });

        await sendMessage(c)({
          ...event.requestContext,
          message: {
            type: MessageType.Complete,
            id: message.id,
          },
        });

        return
      }
andyrichardson commented 3 years ago

Seems straight-forward enough :+1:

Only thing not quite clear is whether there is any case where the error message type should be returned.

Edit: Nevermind - looks like that's maybe for protocol errors.

reconbot commented 3 years ago

I did some testing with graphql-ws

query error

{ message: '{"type":"connection_init","payload":{}}' }
{ send: '{"type":"connection_ack"}' }
{ message: '{"id":"03ffda5b-766d-4c3c-be98-7dd3acf4d907","type":"subscribe","payload":{"query":"query hello {\\n  helloa\\n}\\n\\n\\nsubscription greeting {\\n  greetings\\n}","variables":{},"operationName":"hello"}}' }
{ send: '{"id":"03ffda5b-766d-4c3c-be98-7dd3acf4d907","type":"error","payload":[{"message":"Cannot query field \\"helloa\\" on type \\"Query\\". Did you mean \\"hello\\"?","locations":[{"line":2,"column":3}]}]}' }
Connection closed

resolver error

{ message: '{"type":"connection_init","payload":{}}' }
{ send: '{"type":"connection_ack"}' }
{ message: '{"id":"674df45d-6105-45eb-9739-9950ec3f197c","type":"subscribe","payload":{"query":"query hello {\\n  error\\n}\\n\\n\\nsubscription greeting {\\n  greetings\\n}","variables":{},"operationName":"hello"}}' }
{ send: '{"id":"674df45d-6105-45eb-9739-9950ec3f197c","type":"next","payload":{"errors":[{"message":"Error","locations":[{"line":2,"column":3}],"path":["error"]}],"data":{"error":null}}}' }
{ send: '{"id":"674df45d-6105-45eb-9739-9950ec3f197c","type":"complete"}' }
Connection closed
andyrichardson commented 3 years ago

I would have expected any kind of GraphQL specific response (validation, execution, etc) to be sent in the same message type as a successful response.

@enisdenjo before we go ahead and add support, can we get confirmation that

enisdenjo commented 3 years ago

Yes, you're right.

enisdenjo commented 3 years ago

Do note that the error message is intentional. It does not need to contain validation errors, it can be any "graceful" error that prevents the execution of that specific operation. There are 3 levels of errors in graphql-ws:

  1. Fatal errors that close the connection (breaking the protocol, throwing errors from callbacks, authentication problems, connection issues, etc.)
  2. "Nice" errors that are operation specific and transmitted through the error message (validation, forbidden areas, persisted query misses, etc.)
  3. GraphQL execution errors that get transmitted through the next message because partial results might be available
andyrichardson commented 3 years ago

Thanks for explaining @enisdenjo - I've made a PR to amend this.

andyrichardson commented 3 years ago

@reconbot want to go ahead and make a PR with this change so we can get your contribution attributed?

reconbot commented 3 years ago

Sure, Any ideas on how to reuse the execContext?

andyrichardson commented 3 years ago

@reconbot fair point :thinking:

I've made a PR to expose the utility functions we need but in the meantime, we might need to construct it twice (once in our code and then a second time in graphql-ws when we call "execute").

For the time being, doing it twice should be fine - we'll just need to make sure we only call the context constructor function once.

const contextValue = await constructContext(c)({ connectionParams });
const execContext = buildExecutionContext(
  c.schema,
  parse(message.payload.query),
  undefined,
  contextValue,
  message.payload.variables,
  message.payload.operationName,
  undefined
);

// ...
if (execContext.operation.operation !== 'subscription') {
  const result = await execute(
    c.schema,
    parse(message.payload.query),
    undefined,
    contextValue,
    message.payload.variables,
    message.payload.operationName,
    undefined
  );
reconbot commented 3 years ago

I haven't tested the error states but made a pr with what I got