rails icon indicating copy to clipboard operation
rails copied to clipboard

ActionCable: Subscribe uniquely

Open sj26 opened this issue 2 years ago • 8 comments

If a subscription for the same identifier already exists then it has already been or is being subscribed via the connection, so no need to re-subscribe. Without this change, a subscription message is sent to the server and the guarantor waits for a subscription confirmation which never comes, and so keeps sending subscription messages over and over every second. Fixes #44652.

I'm not sure this is a comprehensive solve, and I'm not sure yet how to test it. I'd love some feedback on the approach. 🙏

I don't think short-circuiting a subscription like this will be a problem. I can't find a place where subscription confirmations are shared back to subscriptions for handling.

sj26 avatar Mar 10 '22 04:03 sj26

I've added another commit which also immediately fires the connected callback for already-subscribed connections. Otherwise such subscriptions might miss initial state setup, etc.

sj26 avatar Mar 21 '22 01:03 sj26

(The tests on CI seem broken for unrelated reasons?)

sj26 avatar Apr 18 '22 22:04 sj26

I don't think short-circuiting a subscription like this will be a problem.

If I understand it correctly, we create two subscription objects, right? The problem may occur if we decide to unsubscribe: the server would unsubscribe the client, while another subscription objet might not know about that and could try to use this stale subscription.

palkan avatar Jun 08 '22 21:06 palkan

There would be multiple client side subscriptions, and one server subscription. When the last client subscription is unsubscribed, the server subscription will also be unsubscribed and removed. If another client subscription is then created, it will create a new server subscription.

sj26 avatar Jul 17 '22 23:07 sj26

If I were architecting the javascript, I would have an M:N layer multiplexing the client and server subscriptions which also manages the subscription process, instead of the subscription guarantor.

sj26 avatar Jul 18 '22 00:07 sj26

Anyone know if @rails/actioncable 6.1.4 works with Rails 7? Just wondering if this bug will hold up us upgrading Rails.

cam-narzt avatar Aug 04 '22 05:08 cam-narzt

Sorry I don't have any updates. I do think this is still an issue. We carry this patch for it in our codebase. But I don't have capacity to push it to completion myself. If someone else is keen, please let me know 🙏

sj26 avatar Sep 10 '23 10:09 sj26