Skip to content
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

Fix broadcast logging in rails 7.1+ #192

Merged
merged 1 commit into from
Oct 15, 2023

Conversation

iuri-gg
Copy link
Contributor

@iuri-gg iuri-gg commented Oct 15, 2023

What is the purpose of this pull request?

Fixes issue with Rails 7.1. Previous fix sets Anycable.logger to an instance of BroadcastLogger with two loggers (console and previous instance of BroadcastLogger). This nested BroadcastLogger breaks with following error:

undefined method `current_tags' for nil:NilClass:
/Users/iuri/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/actioncable-7.1.1/lib/action_cable/connection/tagged_logger_proxy.rb:25:in `tag'
/Users/iuri/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/actioncable-7.1.1/lib/action_cable/connection/tagged_logger_proxy.rb:40:in `log'
/Users/iuri/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/actioncable-7.1.1/lib/action_cable/connection/tagged_logger_proxy.rb:34:in `block (2 levels) in <class:TaggedLoggerProxy>'
/Users/iuri/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/anycable-rails-core-1.4.1/lib/anycable/rails/connection.rb:89:in `handle_open'
...

What changes did you make? (overview)

Use official #broadcast_to API

Is there anything you'd like reviewers to focus on?

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated documentation

@palkan
Copy link
Member

palkan commented Oct 15, 2023

Thanks! Just in time for the release.

@palkan palkan merged commit 61fea33 into anycable:master Oct 15, 2023
12 checks passed
@iuri-gg iuri-gg deleted the fix-broadcast-logger branch October 15, 2023 21:04
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

Successfully merging this pull request may close these issues.

2 participants