Skip to content

Subscriptions with ActionCable do not work with new Interpeter #2495

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

Closed
javiercr opened this issue Sep 25, 2019 · 5 comments
Closed

Subscriptions with ActionCable do not work with new Interpeter #2495

javiercr opened this issue Sep 25, 2019 · 5 comments

Comments

@javiercr
Copy link

javiercr commented Sep 25, 2019

First of all, thank you for all your work on this library!

As it was previously reported, it seems there are some issues with the new Interpreter and ActionCableSubscriptions. This has been reported in #2446, #2483, but those issues didn't provide a failing case.

To do so I've forked the evilmartians/chronicles-gql-martian-library project (that's part of the blog post), and added the minimum set of changes to make it fail: update the graphql gem and add the new interpreter.

Here are the instructions to reproduce the bug

git clone -b graphql-ruby-new-interpreter-bug https://github.com/javiercr/chronicles-gql-martian-library.git
cd chronicles-gql-martian-library
bundle install
rake db:create
rake db:seed
bin/rails s

Now you can log in with john.doe@example.com and add some items. You'll see this in the console:

[ActionCable] Broadcasting to graphql-subscription:30a00d17-d811-4b66-a7de-227f94b01b69: {:result=>{"data"=>nil, "errors"=>[{"message"=>"Cannot return null for non-nullable field Subscription.itemAdded"}]}, :more=>true}
GraphqlChannel transmitting {"result"=>{"data"=>nil, "errors"=>[{"message"=>"Cannot return null for non-nullable field Subscription.itemAdded"}]}, "more"=>true} (via streamed from graphql-subscription:30a00d17-d811-4b66-a7de-227f94b01b69)

As you can see, result.data is nil. It seems that the SubscriptionType field is not receiving any object.

Rolling back to the previous commit (without the new interpeter), everything works perfectly:

[ActionCable] Broadcasting to graphql-subscription:23148bba-f699-4eb5-ba08-06721424e274: {:result=>{"data"=>{"itemAdded"=>{"user"=>{"id"=>"2", "email"=>"jane.doe@example.com", "__typename"=>"User"}, "__typename"=>"Item", "id"=>"10", "title"=>"test", "imageUrl"=>"http://testestest.com", "description"=>"tawdawdawdddd"}}}, :more=>true}
GraphqlChannel transmitting {"result"=>{"data"=>{"itemAdded"=>{"user"=>{"id"=>"2", "email"=>"jane.doe@example.com", "__typename"=>"User"}, "__typename"=>"Item", "id"=>"10", "title"=>"test", "imageUrl"=>"http://testestest.com", "description"=>"tawdawdawdddd"}}}, "more"=>true} (via streamed from graphql-subscription:23148bba-f699-4eb5-ba08-06721424e274)

I hope this helps, as we're looking forward to using the new interpreter and ActionCable subscriptions.

Thank you!

@rmosolgo
Copy link
Owner

rmosolgo commented Sep 26, 2019

Thanks for your work here, @javiercr! I pulled your branch and tried it locally, and I got the same error.

I think the difference is that:

  • Before the interpreter, the return value of subscription fields were actually ignored
  • This reduced customizeability and confused people
  • So, in the interpreter, the return value of subscription fields is used, and passed along for resolving subscription selections

So, the problem is that the def item_added and def item_updated methods return nil, but the field definitions have null: false.

The good news is, those empty methods are no longer required for the interpreter. I solved the problem locally by removing them:

~/code/chronicles-gql-martian-library $ git diff
diff --git a/app/graphql/types/subscription_type.rb b/app/graphql/types/subscription_type.rb
index 56e480c..3e46cbd 100644
--- a/app/graphql/types/subscription_type.rb
+++ b/app/graphql/types/subscription_type.rb
@@ -3,11 +3,8 @@
 module Types
   class SubscriptionType < GraphQL::Schema::Object
     extend GraphQL::Subscriptions::SubscriptionRoot

     field :item_added, Types::ItemType, null: false, description: "An item was added"
     field :item_updated, Types::ItemType, null: false, description: "Existing item was updated"
-
-    def item_added; end
-    def item_updated; end
   end
 end

Does it work for you to remove those methods?

@javiercr
Copy link
Author

javiercr commented Sep 26, 2019

Wow it was that 😅. Yep, it works now both in the test project and in our own private project. That's wonderful.

I guess the docs need an update? Because I've seen other people in the issues struggling with the new Interpreter and ActionCable subscriptions.

Thank you so much for taking a look to this and finding a workaround so quickly!

@rmosolgo
Copy link
Owner

Glad to hear it works for you! You'll find an "Edit this page" link on the docs website, so if you find something to update, please open a PR.

@javiercr
Copy link
Author

@rmosolgo now that the return value of subscription fields are ignored with the new Interpreter, what's the best way to Authorize Subscriptions?

Let's say that I have a Conversation model that has_many Message, and I want to have a subscription every time a message is added:

module Types
  class SubscriptionType < GraphQL::Schema::Object
    extend GraphQL::Subscriptions::SubscriptionRoot

    field :message_added, Types::MessageType, null: false do
      description "A message was added"
      argument :conversation_id, ID, required: true
    end
  end
end

This subscription takes as argument the conversation_id I want to subscribe to, but the return type is a MessageType (the Message object that was added).

If I follow the docs for Authorizing subscriptions, I'd add this method to SubscriptionType:

def message_added(conversation_id:)
  if conversation = ::Conversation.for_user(context[:current_user_id]).find(conversation_id)
    # What to do here?
    # I can't return conversation, as this is not a MessageType
    # but I don't have a message object either.
  else
    raise GraphQL::ExecutionError.new("Can't subscribe to this conversation: #{conversation_id}")
  end
end

How could we authorize the subscription without having to return an object of the type defined for it? (as it seems it was possible before the new Interpreter).

Thanks!

@javiercr
Copy link
Author

Never mind, I just found out about Subscription Classes which I guess it should handle exactly this scenario. Leaving the previous comment here in case it's helpful for someone else :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants