handle sample rate type incompatibilities (#2036)

* cast sample rate values to float

* valid_sample_rate? soft fail

* specs

* check the sample rate's type instead of the methods it supports

Co-authored-by: Stan Lo <stan001212@gmail.com>

* raise ArgumentError unless Numeric/nil

* remove context, more cases

Co-authored-by: Stan Lo <stan001212@gmail.com>

* profiles_sample_rate: more test cases

* CHANGELOG entry

---------

Co-authored-by: Stan Lo <stan001212@gmail.com>

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Move `http.query` to span data in net/http integration [#2039](https://github.com/getsentry/sentry-ruby/pull/2039)
 - Validate `release` is a `String` during configuration [#2040](https://github.com/getsentry/sentry-ruby/pull/2040)
 - Allow JRuby Java exceptions to be captured [#2043](https://github.com/getsentry/sentry-ruby/pull/2043)
+- Improved error handling around `traces_sample_rate`/`profiles_sample_rate` [#2036](https://github.com/getsentry/sentry-ruby/pull/2036)
 
 ### Bug Fixes
 

sentry-ruby/lib/sentry/configuration.rb

@@ -216,8 +216,8 @@ def capture_exception_frame_locals=(value)
  attr_reader :transport
 
  # Take a float between 0.0 and 1.0 as the sample rate for tracing events (transactions).
- # @return [Float]
- attr_accessor :traces_sample_rate
+ # @return [Float, nil]
+ attr_reader :traces_sample_rate
 
  # Take a Proc that controls the sample rate for every tracing event, e.g.
  # @example
@@ -327,7 +327,6 @@ def initialize
  self.before_send = nil
  self.before_send_transaction = nil
  self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT
- self.traces_sample_rate = nil
  self.traces_sampler = nil
  self.enable_tracing = nil
 
@@ -409,7 +408,17 @@ def enable_tracing=(enable_tracing)
  @traces_sample_rate ||= 1.0 if enable_tracing
  end
 
+ def is_numeric_or_nil?(value)
+ value.is_a?(Numeric) || value.nil?
+ end
+
+ def traces_sample_rate=(traces_sample_rate)
+ raise ArgumentError, "traces_sample_rate must be a Numeric or nil" unless is_numeric_or_nil?(traces_sample_rate)
+ @traces_sample_rate = traces_sample_rate
+ end
+
  def profiles_sample_rate=(profiles_sample_rate)
+ raise ArgumentError, "profiles_sample_rate must be a Numeric or nil" unless is_numeric_or_nil?(profiles_sample_rate)
  log_info("Please make sure to include the 'stackprof' gem in your Gemfile to use Profiling with Sentry.") unless defined?(StackProf)
  @profiles_sample_rate = profiles_sample_rate
  end
@@ -443,19 +452,19 @@ def enabled_in_current_env?
  enabled_environments.empty? || enabled_environments.include?(environment)
  end
 
+ def valid_sample_rate?(sample_rate)
+ return false unless sample_rate.is_a?(Numeric)
+ sample_rate >= 0.0 && sample_rate <= 1.0
+ end
+
  def tracing_enabled?
- valid_sampler = !!((@traces_sample_rate &&
- @traces_sample_rate >= 0.0 &&
- @traces_sample_rate <= 1.0) ||
- @traces_sampler)
+ valid_sampler = !!((valid_sample_rate?(@traces_sample_rate)) || @traces_sampler)
 
  (@enable_tracing != false) && valid_sampler && sending_allowed?
  end
 
  def profiling_enabled?
- valid_sampler = !!(@profiles_sample_rate &&
- @profiles_sample_rate >= 0.0 &&
- @profiles_sample_rate <= 1.0)
+ valid_sampler = !!(valid_sample_rate?(@profiles_sample_rate))
 
  tracing_enabled? && valid_sampler && sending_allowed?
  end

sentry-ruby/spec/sentry/configuration_spec.rb

@@ -57,6 +57,29 @@
  end
  end
 
+ describe "#traces_sample_rate" do
+ it "returns nil by default" do
+ expect(subject.traces_sample_rate).to eq(nil)
+ end
+
+ it "accepts Numeric values" do
+ subject.traces_sample_rate = 1
+ expect(subject.traces_sample_rate).to eq(1)
+ subject.traces_sample_rate = 1.0
+ expect(subject.traces_sample_rate).to eq(1.0)
+ end
+
+ it "accepts nil value" do
+ subject.traces_sample_rate = 1
+ subject.traces_sample_rate = nil
+ expect(subject.traces_sample_rate).to eq(nil)
+ end
+
+ it "raises ArgumentError when the value is not Numeric nor nil" do
+ expect { subject.traces_sample_rate = "foobar" }.to raise_error(ArgumentError)
+ end
+ end
+
  describe "#tracing_enabled?" do
  context "when sending not allowed" do
  before do
@@ -154,6 +177,29 @@
  end
  end
 
+ describe "#profiles_sample_rate" do
+ it "returns nil by default" do
+ expect(subject.profiles_sample_rate).to eq(nil)
+ end
+
+ it "accepts Numeric values" do
+ subject.profiles_sample_rate = 1
+ expect(subject.profiles_sample_rate).to eq(1)
+ subject.profiles_sample_rate = 1.0
+ expect(subject.profiles_sample_rate).to eq(1.0)
+ end
+
+ it "accepts nil value" do
+ subject.profiles_sample_rate = 1
+ subject.profiles_sample_rate = nil
+ expect(subject.profiles_sample_rate).to eq(nil)
+ end
+
+ it "raises ArgumentError when the value is not Numeric nor nil" do
+ expect { subject.profiles_sample_rate = "foobar" }.to raise_error(ArgumentError)
+ end
+ end
+
  describe "#profiling_enabled?" do
  it "returns false unless tracing enabled" do
  subject.enable_tracing = false