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 Rails runner's exceptions before exiting #1820

Merged
merged 3 commits into from
May 20, 2022
Merged

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented May 15, 2022

Rails currently doesn't provide any callback for capturing Rails runner's exceptions, so we need to use at_exit for it. But it should be supported in 7.1 (PR: rails/rails#44999) via the enhanced error reporter integration.

And because it's very complicated, if not impossible, to monitor if a Rails runner process reports to Sentry, so I didn't add any test for it. I used this for testing instead:

bundle exec rails runner "1/0" # should be reported
bundle exec rails runner "1/1" # doesn't do anything

Implements and closes #1789

@st0012 st0012 added this to the 5.4.0 milestone May 15, 2022
@st0012 st0012 requested a review from sl0thentr0py May 15, 2022 09:13
@st0012 st0012 added this to In progress in 5.x via automation May 15, 2022
@st0012 st0012 self-assigned this May 15, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 15, 2022

Codecov Report

Merging #1820 (ab8561f) into master (4ba0ca2) will increase coverage by 1.88%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1820      +/-   ##
==========================================
+ Coverage   96.58%   98.46%   +1.88%     
==========================================
  Files         120      145      +25     
  Lines        7373     8661    +1288     
==========================================
+ Hits         7121     8528    +1407     
+ Misses        252      133     -119     
Impacted Files Coverage Δ
sentry-rails/lib/sentry/rails/railtie.rb 97.33% <100.00%> (+71.30%) ⬆️
...rails/tracing/action_controller_subscriber_spec.rb 100.00% <0.00%> (ø)
...entry/rails/tracing/action_view_subscriber_spec.rb 100.00% <0.00%> (ø)
sentry-rails/spec/sentry/rails/event_spec.rb 100.00% <0.00%> (ø)
sentry-rails/spec/sentry/rails/client_spec.rb 100.00% <0.00%> (ø)
...s/lib/sentry/rails/overrides/streaming_reporter.rb 61.53% <0.00%> (ø)
...entry-rails/spec/sentry/rails/action_cable_spec.rb 100.00% <0.00%> (ø)
...ails/breadcrumb/monotonic_active_support_logger.rb 100.00% <0.00%> (ø)
...entry-rails/lib/sentry/rails/controller_methods.rb 100.00% <0.00%> (ø)
...y-rails/lib/sentry/rails/controller_transaction.rb 100.00% <0.00%> (ø)
... and 29 more

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 4ba0ca2...ab8561f. Read the comment docs.

@st0012 st0012 merged commit daeb133 into master May 20, 2022
5.x automation moved this from In progress to Done May 20, 2022
@st0012 st0012 deleted the implement-#1789 branch May 20, 2022 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5.x
Done
Development

Successfully merging this pull request may close these issues.

Capture rails runner's exceptions
3 participants