Graphql-ruby: Execution::Interpreter and Analysis::AST are breaking Subscriptions

[danzanzini]

Hello! Hope you all are good 😄

Recently I had an issue related with #1505. I've used the solution brought here.
So I've added both GraphQL::Execution::Interpreter and the GraphQL::Analysis::AST modules to my Schema.

I'm also using the GraphQL::Subscriptions::ActionCableSubscriptions at my app and after I've added these modules subscriptions stoped working.

When subscriptions are triggered, I don't receive any errors at the back-end but the broser console receives this:

I don't know why this is happening since the subscription works without the GraphQL::Execution::Interpreter and the GraphQL::Analysis::AST modules. I receive the same error for every subscription I execute.

Versions
graphql version: 1.11.5
rails (or other framework): 5.2.2.1

GraphQL schema

Schema before the change:

class MySchema < GraphQL::Schema
  use GraphQL::Subscriptions::ActionCableSubscriptions
  use GraphQL::Pagination::Connections

  mutation(Types::MutationType)
  query(Types::QueryType)
  subscription(Types::SubscriptionType)
end

Schema after the changes:

class ClearWorksSchema < GraphQL::Schema
  use GraphQL::Execution::Interpreter
  use GraphQL::Analysis::AST
  use GraphQL::Subscriptions::ActionCableSubscriptions
  use GraphQL::Pagination::Connections

  mutation(Types::MutationType)
  query(Types::QueryType)
  subscription(Types::SubscriptionType)
end

Sample subscription

class SubscriptionType < Types::BaseObject
    field(
      :conversation_updated,
      Types::ConversationType,
      null: false,
      subscription_scope: :clinic_id,
      description: "A conversation was updated"
    )
end

I don't have much more to add besides this. Any thoughts on why this is happening?
Thank you :)

[dav-armour]

Can confirm this is an issue. We only just figured out this was what was causing our subscriptions to break. Works fine after removing these lines:

use GraphQL::Execution::Interpreter
use GraphQL::Analysis::AST

[dav-armour]

@danzanzini I manage to get mine working with the new interpreter. It required changing how I was setting up my subscriptions. Need to use subscription classes now.
Docs aren't the greatest but this is the relevant part and can hopefully provide some more clarity:
https://graphql-ruby.org/subscriptions/subscription_classes

These are the changes you will most likely need to do:

1. Add a base subscription class

# app/graphql/subscriptions/base_subscription.rb
class Subscriptions::BaseSubscription < GraphQL::Schema::Subscription
  # Hook up base classes
  object_class Types::BaseObject
  field_class Types::BaseField
  argument_class Types::BaseArgument
end

1. Add subscription class for each of your subscriptions

# app/graphql/subscriptions/conversation_updated.rb
class Subscriptions::ConversationUpdated < Subscriptions::BaseSubscription
  description "A conversation was updated"

  payload_type Types::ConversationType
end

1. Update your SubscriptionType class to point to new subscription class

class SubscriptionType < Types::BaseObject
    field :conversation_updated, subscription: Subscriptions::ConversationUpdated
end

Hopefully above helps you out. That is what I had to do to get mine to work.

[rmosolgo]

Hey, thanks for the bug report. I think is is because of how the Interpreter handles subscription fields slightly differently. The old runtime _always_ ignored the value returned by the subscription field, but the interpreter returns the value returned by the field.

The two options I can think of are:

• Switch null: false to null: true and allow the subscription to return null for the initial call.
• Instead of returning nil from the Subscription class, return context.skip. This tells GraphQL-Ruby to _not include_ that field in the response, and it's how subscriptions used to work by default.

Let me know how it goes if you try one of those!

[danzanzini]

Hello, thank you both for the responses.

@dav-armour's solution worked great! It needed the last version of the graphql gem to make it work. :)
Do you mind sharing how you get to this solution? 😬

@rmosolgo I've also tried switching the null: false to null: true but it didn't work. The console error was gone but the subscriptions were still not working.
Do you think we should alert users that classes are required to use the new runtime?

Again, thank you both :D

[rmosolgo]

Thanks for following up, @danzanzini! Did you try returning context.skip?

classes are required to use the new runtime?

It's not my intention to make classes required 🤔 Could you share the body of your conversation_updated method? I wonder why it wasn't working as expected.

[danzanzini]

@rmosolgo I haven't tried it, I didn't understand how to use this option. Can you help me?

Of course, here it is:

module Types
  class SubscriptionType < Types::BaseObject
    field(
      :conversation_updated,
      Types::ConversationType,
      null: false,
      subscription_scope: :clinic_id,
      description: "A conversation was updated"
    )

    field(
      :message_created,
      Types::MessageType,
      null: false,
      subscription_scope: :clinic_id,
      description: "A message was created"
    )

    def patient_updated; end
    def conversation_created; end
  end
end

I have other subscriptions just like these and they all stoped working after the new runtime.

[rmosolgo]

Thanks for sharing. The methods you shared there return nil. (That's the implicit return value of no expression, for example if false; end returnsnil, too).

Here's the code you'd want to put inside each of those methods to make it work the way you're expecting:
https://github.com/rmosolgo/graphql-ruby/blob/c60d7db6046bb4fe2bb1eda2d366b25a748ca8ff/lib/graphql/subscriptions/subscription_root.rb#L18-L22

For example:

def patient_updated
  if context.query.subscription_update? 
    object 
  else 
    context.skip 
  end 
end

(Since the method body is always the same, use resolver_method: option to point all the fields at the same method on the type class. Or use another Ruby code sharing technique, if you want.)

Basically, the difference is that, in old versions of GraphQL-Ruby and on the old runtime, GraphQL-Ruby would _silently ignore_ the return value of those methods. But people were confused and frustrated by that -- they wanted to _use_ that initial value for something. So the interpreter _returned_ the return value of the method. Starting in 1.10 (i think) the old runtime started returning it too (#2294).

But in cases like yours, it becomes broken: You configured your subscription to be null: false, and provided an empty method (which was required at some point IIRC, sorry!). But now, the empty method's nil is being used by GraphQL, which is invalid because of the null: false configuration. Although it seemed broken in this case, for most people it was a "breaking fix" 😅 !

[danzanzini]

@rmosolgo thank you for this clarification. Now everything makes perfect sense.

I've tested the code you sent and it worked great. Thank you. 🎉
I'll use classes because I was about to refactor my subscriptions for a time now and finally the opportunity appeared :)

Good to know that everything is working as expected.
Thank you all for helping me solving this problem ❤️