Skip to content

Commit

Permalink
Use nil instead of false to disable callable settings (#1594)
Browse files Browse the repository at this point in the history
* Use nil instead of false to disable callable settings

* Update changelog
  • Loading branch information
st0012 committed Oct 23, 2021
1 parent 17836c4 commit a87b99d
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

- Connect `Sidekiq`'s transaction with its parent when possible [#1590](https://github.com/getsentry/sentry-ruby/pull/1590)
- Fixes [#1586](https://github.com/getsentry/sentry-ruby/issues/1586)
- Use nil instead of false to disable callable settings [#1594](https://github.com/getsentry/sentry-ruby/pull/1594)

## 4.7.3

Expand Down
20 changes: 10 additions & 10 deletions sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def initialize
self.dsn = ENV['SENTRY_DSN']
self.server_name = server_name_from_env

self.before_send = false
self.before_send = nil
self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT

@transport = Transport::Configuration.new
Expand All @@ -229,9 +229,7 @@ def dsn=(value)


def async=(value)
if value && !value.respond_to?(:call)
raise(ArgumentError, "async must be callable")
end
check_callable!("async", value)

@async = value
end
Expand All @@ -250,17 +248,13 @@ def breadcrumbs_logger=(logger)
end

def before_send=(value)
unless value == false || value.respond_to?(:call)
raise ArgumentError, "before_send must be callable (or false to disable)"
end
check_callable!("before_send", value)

@before_send = value
end

def before_breadcrumb=(value)
unless value.nil? || value.respond_to?(:call)
raise ArgumentError, "before_breadcrumb must be callable (or nil to disable)"
end
check_callable!("before_breadcrumb", value)

@before_breadcrumb = value
end
Expand Down Expand Up @@ -339,6 +333,12 @@ def csp_report_uri

private

def check_callable!(name, value)
unless value == nil || value.respond_to?(:call)
raise ArgumentError, "#{name} must be callable (or nil to disable)"
end
end

def excluded_exception?(incoming_exception)
excluded_exception_classes.any? do |excluded_exception|
matches_exception?(excluded_exception, incoming_exception)
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/spec/sentry/client/event_sending_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
expect(returned).to eq(nil)
end

context "with false as value (the legacy way to disable it)" do
let(:async_block) { false }
context "with nil as value (the legacy way to disable it)" do
let(:async_block) { nil }

it "doesn't cause any issue" do
returned = subject.capture_event(event, scope, { background: false })
Expand Down
18 changes: 15 additions & 3 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,22 @@
end
end

it 'should raise when setting before_send to anything other than callable or false' do
it 'raises error when setting async to anything other than callable or nil' do
subject.async = -> {}
subject.async = nil
expect { subject.async = true }.to raise_error(ArgumentError, "async must be callable (or nil to disable)")
end

it 'raises error when setting before_send to anything other than callable or nil' do
subject.before_send = -> {}
subject.before_send = false
expect { subject.before_send = true }.to raise_error(ArgumentError)
subject.before_send = nil
expect { subject.before_send = true }.to raise_error(ArgumentError, "before_send must be callable (or nil to disable)")
end

it 'raises error when setting before_breadcrumb to anything other than callable or nil' do
subject.before_breadcrumb = -> {}
subject.before_breadcrumb = nil
expect { subject.before_breadcrumb = true }.to raise_error(ArgumentError, "before_breadcrumb must be callable (or nil to disable)")
end

context 'being initialized with a current environment' do
Expand Down

0 comments on commit a87b99d

Please sign in to comment.