Graphql-ruby: ActionCableLink does not send unsubscribe message to Action Cable

Created on 20 Mar 2020  路  7Comments  路  Source: rmosolgo/graphql-ruby

Describe the bug

Subscriptions set up with subscribeToMore in Apollo are successfully connecting and recieving messages, however when unsubscribing (by manually calling the unsubscribe function, or letting the component's query clean itself up) the unsubscribe message is not being sent to the backend. This leaves the subscription open on the backend, resulting in messages being sent on the cable with nothing to consume them.

Versions

Rails dependencies:
graphql: 1.10.3
rails: 6.0.2.1
graphql-batch: 0.4.2

Javascript dependencies:
graphql: 14.5.8
graphql-ruby-client: 1.7.4
actioncable: 5.2.4-1
apollo-client: 2.6.4
apollo-link: 1.2.13

Steps to reproduce

  • Call subscribeToMore in getDerivedStateFromProps in a React component
  • Store the result of subscribeToMore in state.unsubscribe
  • call this.state.unsubscribe in componentWillUnmount

Expected behavior

A "command": "unsubscribe" message should be sent on /cable

Actual behavior

No message is sent. Subsequent subscription triggers on the backend continue to send messages to the frontend, although nothing is consuming those messages

Additional context

I noticed that if the message payload contains "more": false the subscription properly unsubscribes, however if the client initiates the unsubscribe nothing happens

Most helpful comment

@rmosolgo any chance we can get a release with this fix?

All 7 comments

Thanks for reporting this issue. Sorry about the bug! I _thought_ this line would export an .unsubscribe() function for Apollo to call:

https://github.com/rmosolgo/graphql-ruby/blob/5926cacd409a8d5c1b46fc259a23363c51b6c22c/javascript_client/src/subscriptions/ActionCableLink.ts#L56-L57

But maybe it doesn't. That line could be improved by explicitly defining functions on the returned object.

Did you find a way to modify it that worked for you?

I hadn't found a workaround but I did test out explicitly exporting .unsubscribe() by doing {...subscription, unsubscribe: subscription.unsubscribe} on this line and it seems to work. I'll submit a PR for the fix.

weird! I assumed that ...subcription would have exported unsubscribe. Glad to hear that it works. I wonder why it doesn't do that.

Ahhh... I'm reading now that ...obj doesn't pass along obj's prototype, so any "inherited" properties aren't passed along. I bet that's the problem!

Yep just discovered that myself. The test in ActionCableLinkTest.ts is passing because it is using Object.assign to set perform and unsubscribe. Using Object.create should set these as prototype functions. I'll update both the test and code to reflect this.

@rmosolgo any chance we can get a release with this fix?

Just shipped 1.7.6 with these changes. Thanks for the bump, and @davidpimentel , thanks for the fix!

Was this page helpful?
0 / 5 - 0 ratings