Allow intercepting exceptions that are raised in ActionCable command #34917
Conversation
|
Is there a way I could get a maintainer to weigh in on this PR? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
Rails bot just reminded me this hasn't been addressed. I still think this is something that should be supported by ActionCable if not this exact PR. It would be great to get a Rails maintainer to weigh in |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This is a very useful contribution, could someone please take a look? |
There was a problem hiding this comment.
Thanks @waymondo!
Let's use ActiveSupport::Rescuable.
Check out https://github.com/rails/rails/pull/34686/files for an example of the usage style.
|
Thanks @jeremy. That's an obvious improvement in retrospect. Just updated the PR. |
There was a problem hiding this comment.
Looking good!
Noting that a Channel rescue_from would handle the exception example in the test case. In this sense, setting a Connection rescue_from is a convenient fallback, in addition to catching other unhandled exceptions.
Let's update the changelog as well!
|
Yeah, I only learned since you linked that PR as an example that the |
|
Same git repo! Guides can be updated alongside this PR, if you're interested: https://github.com/rails/rails/blob/master/guides/source/action_cable_overview.md |
and update ActionCable guide to describe exception handling usage # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch master # Your branch is behind 'origin/master' by 5 commits, and can be fast-forwarded. # # Changes to be committed: # modified: actioncable/CHANGELOG.md # modified: actioncable/lib/action_cable/connection/base.rb # modified: actioncable/lib/action_cable/connection/subscriptions.rb # modified: actioncable/test/connection/subscriptions_test.rb # modified: guides/source/action_cable_overview.md #
|
Cool, updated the guides and combined everything into a single commit. Let me know if there's anything else you'd like me to adjust. |
|
|
||
| By default, unhandled exceptions are caught and logged to Rails' logger. If you would like to | ||
| globally intercept these exceptions and report them to an external bug tracking service, for | ||
| example, you can do so with `rescue_with`. |
There was a problem hiding this comment.
It looks like we got the method name and the with parameter mixed up:
| example, you can do so with `rescue_with`. | |
| example, you can do so with `rescue_from`. |
| #### Exception Handling | ||
|
|
||
| As with `ActionCable::Connection::Base`, you can also use | ||
| [`rescue_with`](https://api.rubyonrails.org/classes/ActiveSupport/Rescuable/ClassMethods.html) |
There was a problem hiding this comment.
Same thing here:
| [`rescue_with`](https://api.rubyonrails.org/classes/ActiveSupport/Rescuable/ClassMethods.html) | |
| [`rescue_from`](https://api.rubyonrails.org/classes/ActiveSupport/Rescuable/ClassMethods.html) |
|
I noticed that this implementation doesn't rescue exceptions raised in Edit: Nevermind. I figured this out. For the subscribe/unsubscribe events, the |
Summary
Exceptions raised during command execution on an ActionCable Channel
are logged and suppressed, which does not allow for any sort of error
handling or reporting (via external services such as Bugsnag or Honeybadger).
This adds a simple hook to intercept the rescued exception on the ActionCable Connection.
Other Information
I had thought that maybe the default implementation of
on_command_execution_exceptionshould be the current logging behavior, but I opted for it being a no op. I'm happy to change it if that is preferable.This is my first Rails PR so please let me know if there are any conventions that I should change about this patch.