getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
927 stars 493 forks source link

Fix NoMethodError / Make session_tracking check consistent #2269

Closed knapo closed 5 months ago

knapo commented 5 months ago

Description

https://github.com/getsentry/sentry-ruby/pull/2245 (cc @st0012 ) introduced changes around checking if session_tracking is enabled or not.. However, it wasn't updated in all necessary places and make it inconsistent within the codebase. As result, when not enabled_in_current_env? we're getting a following error:

     NoMethodError:
       undefined method `add_session' for nil:NilClass
     # <REDACTED>/gems/sentry-ruby-5.17.0/lib/sentry/hub.rb:244:in `end_session'
     # <REDACTED>/gems/sentry-ruby-5.17.0/lib/sentry/hub.rb:253:in `with_session_tracking'
     # <REDACTED>/gems/sentry-ruby-5.17.0/lib/sentry-ruby.rb:403:in `with_session_tracking'

Affected version: sentry-ruby 5.17.0

codecov[bot] commented 5 months ago

Codecov Report

Merging #2269 (9a84215) into master (b05afd9) will not change coverage. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2269 +/- ## ======================================= Coverage 97.61% 97.61% ======================================= Files 112 112 Lines 4155 4155 ======================================= Hits 4056 4056 Misses 99 99 ``` | [Components](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `98.30% <100.00%> (ø)` | | | [sentry-rails](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `95.05% <ø> (ø)` | | | [sentry-sidekiq](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.70% <ø> (ø)` | | | [sentry-resque](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `92.30% <ø> (ø)` | | | [sentry-delayed_job](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `95.60% <ø> (ø)` | | | [sentry-opentelemetry](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `100.00% <ø> (ø)` | | | [Files](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby/lib/sentry/hub.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2269?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXJ1YnkvbGliL3NlbnRyeS9odWIucmI=) | `99.38% <100.00%> (ø)` | |
ixti commented 5 months ago

Still getting this error on Ruby-3.3:


      NoMethodError:
        undefined method `add_session' for nil
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry/hub.rb:244:in `end_session'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry/hub.rb:253:in `with_session_tracking'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry-ruby.rb:403:in `with_session_tracking'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry/rack/capture_exceptions.rb:19:in `block in call'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry/hub.rb:59:in `with_scope'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry-ruby.rb:383:in `with_scope'
      # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.17.1/lib/sentry/rack/capture_exceptions.rb:18:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/rails_semantic_logger-4.14.0/lib/rails_semantic_logger/rack/logger.rb:45:in `call_app'
      # /home/ixti/.gem/ruby/3.3.0/gems/rails_semantic_logger-4.14.0/lib/rails_semantic_logger/rack/logger.rb:26:in `block in call'
      # /home/ixti/.gem/ruby/3.3.0/gems/semantic_logger-4.15.0/lib/semantic_logger/base.rb:190:in `block in tagged'
      # /home/ixti/.gem/ruby/3.3.0/gems/semantic_logger-4.15.0/lib/semantic_logger/semantic_logger.rb:395:in `named_tagged'
      # /home/ixti/.gem/ruby/3.3.0/gems/semantic_logger-4.15.0/lib/semantic_logger/base.rb:197:in `tagged'
      # /home/ixti/.gem/ruby/3.3.0/gems/rails_semantic_logger-4.14.0/lib/rails_semantic_logger/rack/logger.rb:26:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/rack-3.0.9.1/lib/rack/method_override.rb:28:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/rack-3.0.9.1/lib/rack/runtime.rb:24:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/rack-3.0.9.1/lib/rack/sendfile.rb:114:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/ddtrace-1.21.0/lib/datadog/tracing/contrib/rack/middlewares.rb:113:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/railties-7.1.3.2/lib/rails/engine.rb:536:in `call'
      # /home/ixti/.gem/ruby/3.3.0/gems/rack-test-2.1.0/lib/rack/test.rb:360:in `process_request'
      # /home/ixti/.gem/ruby/3.3.0/gems/rack-test-2.1.0/lib/rack/test.rb:153:in `request'
      # ./spec/support/helpers/request_helpers.rb:7:in `login'
      # ./spec/requests/onboardings_controller_spec.rb:55:in `block (4 levels) in <top (required)>'
      # /home/ixti/.gem/ruby/3.3.0/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

UPDATE: I believe that's due to my misconfiguration, as this error only occurs during specific order of specs execution.

ixti commented 5 months ago

Figured out the reason for https://github.com/getsentry/sentry-ruby/pull/2269#issuecomment-2002596748

It happens when we do in specs:

require "sentry/test_helper"

RSpec.configure do |config|
  config.include Sentry::TestHelper, sentry_test: true
  config.before(sentry_test: true) { setup_sentry_test }
  config.after(sentry_test: true) { teardown_sentry_test }
end
ixti commented 5 months ago

Okay. Found how to reproduce the error:

Sentry.init do |config|
  # ...

  config.enabled_environments = %w[staging production]

  # ...
end

Then using in specs:

require "sentry/test_helper"

RSpec.configure do |config|
  config.include Sentry::TestHelper
end

RSpec.describe "..." do
  before { setup_sentry_test }
  after { teardown_sentry_test }

  # ...
end

setup_sentry_test injects "test" into environments