excid3 / noticed

Notifications for Ruby on Rails applications
MIT License
2.44k stars 173 forks source link

Rails crashes when not using ActionCable on 2.0.2 and above. #392

Closed btwelch-corp closed 9 months ago

btwelch-corp commented 9 months ago

Bug Report

Describe the Bug: In rails 7, while not using action_cable as a dependency, when upgrading from 2.0.0 to 2.0.6, I'm getting:

NameError: uninitialized constant ApplicationCable (NameError) class NotificationChannel < ::ApplicationCable::Channel

... when running rails s or migrations.

To Reproduce: Remove action_cable as dependency in rails project.

  1. Remove action_cable as dependency in rails project
  2. Update gemfile to use Noticed 2.0.2 or higher
  3. Run rails s

Expected Behavior: The app should run without error.

Actual Behavior: I get the error: NameError: uninitialized constant ApplicationCable (NameError) class NotificationChannel < ::ApplicationCable::Channel

Screenshots (if applicable): n/a

Environment:

Additional Context: Everything works fine on 2.0.1

Possible Fix: n/a

Steps to Reproduce with Fix (if available): n/a

Related Issues: n/a

Labels to Apply: n/a

Checklist:

excid3 commented 9 months ago

Please provide the full stacktrace.

The provided steps don't reproduce the issue.

rails new --skip-action-cable example-app
cd example-app 
bundle add noticed
rails s

=> Booting Puma
=> Rails 7.1.3 application starting in development
=> Run `bin/rails server --help` for more startup options
Puma starting in single mode...
* Puma version: 6.4.2 (ruby 3.3.0-p0) ("The Eagle of Durango")
*  Min threads: 5
*  Max threads: 5
*  Environment: development
*          PID: 54418
* Listening on http://127.0.0.1:3000
* Listening on http://[::1]:3000
Use Ctrl-C to stop
Started GET "/" for ::1 at 2024-01-29 17:47:46 -0600
  ActiveRecord::SchemaMigration Load (0.1ms)  SELECT "schema_migrations"."version" FROM "schema_migrations" ORDER BY "schema_migrations"."version" ASC
Processing by Rails::WelcomeController#index as HTML
  Rendering /Users/chris/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/railties-7.1.3/lib/rails/templates/rails/welcome/index.html.erb
  Rendered /Users/chris/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/railties-7.1.3/lib/rails/templates/rails/welcome/index.html.erb (Duration: 0.4ms | Allocations: 374)
Completed 200 OK in 5ms (Views: 1.6ms | ActiveRecord: 0.0ms | Allocations: 4296)
stephannv commented 9 months ago

This happens because you project doesn't have a ApplicationCable::Channel class, you must create it:

# app/channels/application_cable/channel.rb
module ApplicationCable
  class Channel < ActionCable::Channel::Base
  end
end

@excid3 I'm not sure the best solution here, but this works:

# app/channels/noticed/notification_channel.rb
module Noticed
  class NotificationChannel < Object.const_defined?("ApplicationCable::Channel") ? ::ApplicationCable::Channel : ActionCable::Channel::Base
    def subscribed
      stream_for current_user
    end

    def unsubscribed
      stop_all_streams
    end

    def mark_as_seen(data)
      current_user.notifications.where(id: data["ids"]).mark_as_seen
    end

    def mark_as_read(data)
      current_user.notifications.where(id: data["ids"]).mark_as_read
    end
  end
end
excid3 commented 9 months ago

Right, but creating an app without ActionCable did not fail so I need complete reproduction steps. Maybe enabling eager loading would reproduce it.

stephannv commented 9 months ago

Right, but creating an app without ActionCable did not fail so I need complete reproduction steps. Maybe enabling eager loading would reproduce it.

Yeah @excid3, on my fresh new app, I could not reproduce this error when I delete the app/channels folder, I can start the app (rails console & server and run tests) and I can send notifications with no problems. But on my job app (very old rails app), it couldn't start it (console, server or tests), the error reported happens. But on my fresh new app if I run Noticed::NotificationChannel on console, the error is raised:

❯ rails c
Loading development environment (Rails 7.1.3)
irb(main):001> Noticed::NotificationChannel
/Users/stephann/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/noticed-2.1.0/app/channels/noticed/notification_channel.rb:4:in `<module:Noticed>': uninitialized constant ApplicationCable (NameError)

  class NotificationChannel < ::ApplicationCable::Channel
                              ^^^^^^^^^^^^^^^^^^
Did you mean?  ApplicationJob
               ApplicationMailer
               ApplicationPage
irb(main):002>
excid3 commented 9 months ago

Yep. We just need to figure out what's causing it to load that file. If ActionCable isn't there, it shouldn't be referencing that class. Only thing I can think of is eager loading for production which is why development wouldn't replicate it.

excid3 commented 9 months ago

Confirmed eager_load = true reproduces it.