-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Rails 7 compatibility issue #165
Comments
That could happen if AnyCable extensions haven't been activated; what is the AnyCable works with Rails 7 fine. |
In Ruby 3.1 output is more detailed:
|
So for me, it seems like in my app AnyCable somehow failing to load/monkey patch ActionCable but I cannot figure out a good way to tap into that to debug why. |
At the same time socket and env are overridden by AnyCable. |
@palkan I noticed you changing Monkey Patching in your Lograge fork and just in case I have it in Gemfile for this problematic application. But excluding it from Gemfile does not mitigate the problem. |
Any chance of that issue being due to Zeitwerk?
I patched
|
I can even get one step further with entry-point like that:
But then it become even more odd:
|
I'm having the same problem, but after upgrading from Rails 6.0 to Rails 6.1(.4.4) with This would indicate that indeed it's not the Rails upgrade but something else. I haven't dug any deeper yet, but just FYI. |
@jdongelmans Thanks for sharing! And what about your Ruby version? |
@palkan No problem and sorry 'bout that. Issues are present on both Ruby
|
Couldn't reproduce this problem using the latest version (of everything) so far. Does dropping |
Adding before
Adding right after |
I was able to push Zeitwerk patch further. But it still far from over:
|
I don't think hacking around with Zeitwerk is a way. AnyCable doesn't rely on it to load the patches. This way we can find when and why each file is loaded. If it shows that Rails' version is loaded multiple times (and after the AnyCable patch), that would explain this. |
Hi @palkan, Thanks for looking into this a bit deeper. I agree, we should not work around Zeitwerk as this would probably give more headaches in the future. I have attached the requested output for my project. Click for log
edit: I saw bootsnap in the logs and figured that might also be something that could mess up the loading, so I've tried reverting that to the last working version (1.7.3) and I've tried removing it completely, but still same issue unfortunately 😞 |
I am a bit amazed by such reaction for Zeitwerk, which is currently defacto standard in Ruby with Rails 7 release. It gives headaches when you break conventions and even then you have quite a lot of control. But aside from that here is my logsaction_cable/connection/base.rb:
anycable/rails/actioncable/connection.rb:
|
Thanks @bopm. So, each file is loaded just once? The order is correct. Okay, let's try another trick: could you please open the
I believe, we should see the |
That helped:
|
@jdongelmans are you using Sentry as well? |
Nice find. Indeed Sentry is also in this project and after removing it the websocket connects no problems at all 🎉 |
@palkan while we get to the bottom of that, disabling Sentry is not a solution for me. What are the next steps for this issue? |
@bopm for now we've reverted all - gem 'sentry-rails', '5.0.1'
- gem 'sentry-ruby', '5.0.1'
- gem 'sentry-sidekiq', '5.0.1'
+ gem 'sentry-rails', '4.8.3'
+ gem 'sentry-ruby', '4.8.3'
+ gem 'sentry-sidekiq', '4.8.3' running rails 6.1.4.4 and ruby 2.7.5 |
Same problem here. But, +1 to @bopm's question though, This seems like an issue with anycable-rails, no? |
@bopm thanks for tracking this, i started going down this rabbit hole thinking it was related to my changes for https://docs.anycable.io/anycable-go/signed_streams ruby 3.1 + edge rails 7 + turbo ... i also use sentry. Has anyone filed anything with sentry yet? |
Is there anything for Sentry to even do here? |
oops, still catching up. Just saw the last stack frame inside the gem and the version downgrade and made the wrong assumption. oh 👋 david, makes me happy (and oddly nostalgic?) to see you active here |
@rromanchuk @davidbyttow to my understanding (but I really like to hear @palkan opinion) AnyCable is doing the most terrifying thing in monkey patching of preexisting base API methods visibility. The only way to make it worse is to make something public on that API private inside of AnyCable . |
Thanks @bopm!
I wouldn't use "the most" though 🙂 The reasoning behind this is in keeping compatibility with our spec. Action Cable is just one of the possible adapters for AnyCable, and
Nope, their patch is 👍 Looks like it makes sense to spend a bit more time and get rid of all the monkey-patching; stay tuned! |
@palkan don't get me wrong, AnyCable is amazing piece of Rails infrastructure. Probably, correct way is to get Rails Core involved and make those methods public. Problem here is only with making them public all the sudden. |
As a quick fix, I suggest adding the following somewhere in the configuration: # in config/application.rb
initializer "anycable.sentry_hack", after: :"sentry.extend_action_cable" do
next unless defined?(::Sentry::Rails::ActionCableExtensions::Connection)
Sentry::Rails::ActionCableExtensions::Connection.send :public, :handle_open, :handle_close
end |
Hmm this was no change for me, not sure how thats possible. I'm going to step through it on production and see what I can figure out. |
You're right. Updated the patch. |
Released 1.2.1 with a temporary workaround. Check it, please, and let me know if the problem still persists. |
@palkan i'm all over the place right now, so take this with a grain of salt I'm noticing this coming up in staging currently
I'm on |
@rromanchuk Lograge PR was recently accepted, return to the upstream from the fork. |
Everything looks healthy now 👍 , thanks
|
The hack (and the patched 1.2.1 release) don't correctly work on Rails 6.1.4.4 / Ruby 3.1.0 / Sentry-rails 5.0.1. The initializer doesn't get the proper order with I guess it's fine for a temporary hack. Worked around it by putting the proper initializer in |
Oh. Does moving |
This PR changes the (historical) way of making Action Cable compatible with AnyCable: - Extend (via #prepend) instead of reopening classes - Preserve the original functionality (by using #anycabled? checks where necessary) Closes #165
This PR changes the (historical) way of making Action Cable compatible with AnyCable: - Extend (via #prepend) instead of reopening classes - Preserve the original functionality (by using #anycabled? checks where necessary) Closes #165
This PR changes the (historical) way of making Action Cable compatible with AnyCable: - Extend (via #prepend) instead of reopening classes - Preserve the original functionality (by using #anycabled? checks where necessary) Closes #165
This PR changes the (historical) way of making Action Cable compatible with AnyCable: - Extend (via #prepend) instead of reopening classes - Preserve the original functionality (by using #anycabled? checks where necessary) Closes #165
The v1.3.0 has been released with the rewritten Action Cable integration, which should help us to prevent such issues in the future. |
@bopm i'm getting below now. is there an easy way to disable this lograge/actioncable integration completely? Ive been playing wack-a-mole between 3 different gems now for too long
lograge (bf6954566607) lib/lograge/rails_ext/action_cable/channel/base.rb in subscribe_to_channel at line 6 |
Thanks for reporting! Released 1.3.1 with a fix. Please, let me know if there are any other issues. I'd like to make sure we find all the edge cases with the conflicting patches, that would be helpful for everyone. |
Tell us about your environment
Ruby version: 2.7/3.1
Rails version: 7.0.1
anycable
gem version: 1.2.0anycable-rails
gem version: 1.2.0grpc
gem version: 1.42.0What did you do?
Upgrade application to Rails 7, run application.
What did you expect to happen?
AnyCable keeps working.
What actually happened?
AnyCable is not working with error:
The text was updated successfully, but these errors were encountered: