Skip to content

Register Sentry's ErrorSubscriber for Rails 7.0+ apps#1705

Merged
st0012 merged 4 commits intomasterfrom
implement-#1630
Feb 1, 2022
Merged

Register Sentry's ErrorSubscriber for Rails 7.0+ apps#1705
st0012 merged 4 commits intomasterfrom
implement-#1630

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Jan 30, 2022 •
edited
Loading

This implements the integration proposed in #1630.

After Rails 7.0, you'll be able to use the standardized Rails.error.record or Rails.error.handle interfaces to report exceptions to Sentry. Please check ActiveSupport::ErrorReporter for more information.

Gotcha & Workaround

If an operation is wrapped inside app.executor.wrap, any exception raised will be captured by Rails' error reporter,
even if it has been reported by either the SDK integration or user. So in some cases (e.g. ActionCable's channel actions), the same error could be reported twice.

I worked around this by marking if an exception has been captured by the SDK. And if it has, we don't capture it with the ErrorSubscriber.

(Dropping the original integration isn't an option because the contextual data may not be accessible from the error reporter)

Closes #1630

@st0012 st0012 added this to the 5.1.0 milestone Jan 30, 2022
@st0012 st0012 self-assigned this Jan 30, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 30, 2022 •
edited
Loading

Codecov Report

Merging #1705 (af3d659) into master (4217838) will decrease coverage by 0.17%.
The diff coverage is 39.13%.

❗ Current head af3d659 differs from pull request most recent head 63225be. Consider uploading reports for the commit 63225be to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1705      +/-   ##
==========================================
- Coverage   98.59%   98.41%   -0.18%     
==========================================
  Files         136      136              
  Lines        7808     7830      +22     
==========================================
+ Hits         7698     7706       +8     
- Misses        110      124      +14
Impacted Files Coverage Δ
sentry-rails/spec/sentry/rails_spec.rb 90.69% <25.00%> (-9.31%) ⬇️
sentry-rails/lib/sentry/rails/railtie.rb 97.26% <50.00%> (-2.74%) ⬇️
sentry-rails/spec/support/test_rails_app/app.rb 97.76% <100.00%> (+0.01%) ⬆️
sentry-ruby/lib/sentry/hub.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4217838...63225be. Read the comment docs.

@st0012 st0012 force-pushed the implement-#1630 branch 2 times, most recently from f136ad6 to c9d107a Compare January 30, 2022 18:08
@st0012
Copy link
Copy Markdown
Contributor Author

st0012 commented Jan 30, 2022

@georgeclaghorn I'd like to hear your opinion on the gotcha and the workaround I took 🙂

@st0012 st0012 requested a review from sl0thentr0py January 30, 2022 18:11
@georgeclaghorn
Copy link
Copy Markdown

georgeclaghorn commented Jan 30, 2022

I don't feel familiar enough with this gem to give a thorough review but this all seems reasonable to me. 👍

sl0thentr0py
sl0thentr0py approved these changes Jan 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

cool that we support this now!

@st0012 st0012 merged commit 9889bbe into master Feb 1, 2022
@st0012 st0012 deleted the implement-#1630 branch February 1, 2022 10:19
lewispb added a commit to lewispb/sentry-ruby that referenced this pull request Feb 1, 2022
@lewispb
Merge branch 'master' into lb/redis
a2b63bb 
* master:
  feat(performance): Sync activerecord and net-http span names (getsentry#1681)
  Register Sentry's ErrorSubscriber for Rails 7.0+ apps (getsentry#1705)
  Support serializing ActiveRecord job arguments in global id form (getsentry#1688)
  release: 5.0.2
  Fix report_after_job_retries's decision logic (getsentry#1704)
  Followup of getsentry#1701 (getsentry#1703)
  Capture transaction tags (getsentry#1701)
@st0012 st0012 mentioned this pull request Feb 17, 2022
Closed
@casperisfine
Copy link
Copy Markdown
Contributor

casperisfine commented Feb 18, 2022

(Dropping the original integration isn't an option because the contextual data may not be accessible from the error reporter)

Could you expand on that? I'm not sure I follow.

@ariccio
Copy link
Copy Markdown

ariccio commented Sep 12, 2023

Interesting. I think this means I can turn on config.active_record.db_warnings_action, setting it to :report, and it will automatically send them through to sentry? Awesome! I'll have to try it!

@trevorturk trevorturk mentioned this pull request Feb 7, 2024
Closed
jgraichen added a commit to openmensa/openmensa that referenced this pull request Jul 19, 2025
@jgraichen
chore: Use Rails builtin error reporting
84684ee 
Sentry and AppSignal automatically hooks into the new error reporting
framework in Rails.

* https://guides.rubyonrails.org/error_reporting.html
* getsentry/sentry-ruby#1705
* https://blog.appsignal.com/2023/06/20/exceptional-error-reporting-for-rails-exceptions.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sl0thentr0py sl0thentr0py sl0thentr0py approved these changes

Assignees

@st0012 st0012

Labels

sentry-rails

Projects

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

Support the new Rails standard error reporting interface

6 participants

@st0012 @codecov-commenter @georgeclaghorn @casperisfine @ariccio @sl0thentr0py