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

Capture exceptions in Action Cable connections and channels (continuation of #1295) #1638

Merged
merged 9 commits into from
Dec 12, 2021

Conversation

choznerol
Copy link
Contributor

@choznerol choznerol commented Dec 9, 2021

Description

Close #470

This is a continuation of #1295 (#1295 (comment))

Most of the features are already done in #1295, I only added:

  1. Update according to latest review comments Capture exceptions in Action Cable connections and channels #1295 (review).
  2. Capture exceptions in Action Cable connections and channels #1295 integrates with Rails similar to how Sentry::Rails::ActiveJobExtensions did. When resolving conflict with the latest master, however, I realized the latter has changed a lot in Only extend ActiveJob when it's defined #1218 and Use ActiveSupport Lazy Load Hook to Apply ActiveJob Extension #1494. I try to update Sentry::Rails::ActionCableExtensions in a similar fashion to prevent bugs like sentry + eager_load breaks ActiveJob rescue_from #1249.

There are 5 types of hooks that Action Cable provides:

* `Connection#connect`
* `Connection#disconnect`
* `Channel#subscribed`
* `Channel#unsubscribed`
* `Channel` actions

Now, any exceptions raised within those hooks are captured by Sentry, and reported as `ActionCable/[...]` transactions. Additional context is included depending on the hook the exception was raised within.

A note/quirk: the Rack env that's included in the scope is from the `Connection`, and therefore has a URL of the cable `mount_path` (usually `/cable`) as well as the headers from that initial connection request.

Additionally, there is not currently a really clean way to hook in and set `user_context`. I don't know if that is a blocker for this integration, but wanted to make sure it was noted.
choznerol added a commit to choznerol/sentry-ruby that referenced this pull request Dec 9, 2021
choznerol added a commit to choznerol/sentry-ruby that referenced this pull request Dec 9, 2021
choznerol added a commit to choznerol/sentry-ruby that referenced this pull request Dec 9, 2021
@choznerol choznerol marked this pull request as ready for review December 9, 2021 15:16
Copy link
Collaborator

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@choznerol @agrobbin thanks for adding this feature!

@st0012 st0012 merged commit ce10521 into getsentry:master Dec 12, 2021
@choznerol choznerol deleted the rails-action-cable-support branch December 13, 2021 01:47
@agrobbin
Copy link
Contributor

Awesome, thanks @choznerol @st0012!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActionCable probably doesn't work
3 participants