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

Disable background worker when executing rake tasks #1509

Merged
merged 3 commits into from
Jul 23, 2021
Merged

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Jul 22, 2021

This fixes #1508

(I know disabling background worker by setting background_worker_threads = 0 looks hacky, but I don't want to introduce another config option for background toggling atm.)

@st0012 st0012 added this to the 4.7.0 milestone Jul 22, 2021
@st0012 st0012 self-assigned this Jul 22, 2021
@st0012 st0012 added this to In progress in 4.x via automation Jul 22, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2021

Codecov Report

Merging #1509 (3406ed1) into master (157f958) will increase coverage by 0.52%.
The diff coverage is 92.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1509      +/-   ##
==========================================
+ Coverage   98.21%   98.74%   +0.52%     
==========================================
  Files         218      123      -95     
  Lines       10560     6761    -3799     
==========================================
- Hits        10372     6676    -3696     
+ Misses        188       85     -103     
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/rake.rb 52.94% <50.00%> (-1.61%) ⬇️
sentry-ruby/lib/sentry/hub.rb 100.00% <100.00%> (ø)
sentry-ruby/spec/sentry/hub_spec.rb 100.00% <100.00%> (ø)
...ven/lib/raven/breadcrumbs/active_support_logger.rb
sentry-raven/lib/raven/base.rb
...y-raven/spec/raven/processors/http_headers_spec.rb
sentry-raven/spec/raven/transports/http_spec.rb
sentry-raven/spec/raven/event_spec.rb
...try-raven/lib/raven/interfaces/single_exception.rb
sentry-raven/lib/raven/event.rb
... and 88 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 157f958...3406ed1. Read the comment docs.

@y-yagi
Copy link
Contributor

y-yagi commented Jul 23, 2021

@st0012 Thanks for your quick fix! But, unfortunately, this can't fix my issue.

Rake::Application#top_level calls before the initialization process of a Rails application. So if users configure Sentry inside an initializer(that mentioned in doc), Sentry.initialized? inside Rake::Application#top_level returns false always and not using Sentry.get_current_hub.with_background_worker_disabled.

@st0012
Copy link
Collaborator Author

st0012 commented Jul 23, 2021

@y-yagi I see. I now see that the Rails rake task's initialization order is like this:

Rails rake_tasks block
top_level
Sentry SDK initializer
inside the task

I'll dig into this more.

@st0012
Copy link
Collaborator Author

st0012 commented Jul 23, 2021

@y-yagi I've updated the fix. It should work for both vanilla rake and Rails rake tasks.

This helper temporarily disables background event dispatching when
executing the block.
This makes sure all events, whether manually triggered or handled by the
SDK, can be sent synchronously.
@y-yagi
Copy link
Contributor

y-yagi commented Jul 23, 2021

@st0012 Thanks! I confirmed this fixes my issue.

@st0012
Copy link
Collaborator Author

st0012 commented Jul 23, 2021

@y-yagi sorry for the trouble and thanks for helping me verify the fix 🙂

@st0012 st0012 merged commit 4dc603b into master Jul 23, 2021
4.x automation moved this from In progress to Done Jul 23, 2021
@st0012 st0012 deleted the fix-#1508 branch July 23, 2021 06:37
@st0012 st0012 modified the milestones: 4.7.0, 4.6.2 Jul 23, 2021
@y-yagi
Copy link
Contributor

y-yagi commented Jul 23, 2021

@st0012 No problem! Thanks for your quick fix ❤️

@st0012
Copy link
Collaborator Author

st0012 commented Jul 23, 2021

@y-yagi fyi it's been released in version 4.6.2

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

Successfully merging this pull request may close these issues.

Sentry.capture_message can't send a message from Rake
3 participants