Skip to content

Commit

Permalink
Delete NilNotifiers and rely on notifier config checks
Browse files Browse the repository at this point in the history
When a notifier is not configured, we can `add_filter` but `notify` won't have
effect. This accompanies #445.
  • Loading branch information
kyrylo committed Feb 28, 2019
1 parent c043a6c commit fe56299
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 72 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ Airbrake Ruby Changelog

### master

* `add_filter` & `add_performance_filter` add filters even when Airbrake is not
configured ([#445](https://github.com/airbrake/airbrake-ruby/pull/445))

### [v4.0.1][v4.0.1] (Feburary 26, 2019)

* Fixed bug in `Airbrake.configure` not setting `logger` properly
Expand Down
78 changes: 6 additions & 72 deletions lib/airbrake-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,71 +77,9 @@ module Airbrake
# @!macro see_public_api_method
# @see Airbrake.$0

# NilNoticeNotifier is a no-op notice notifier, which mimics
# +Airbrake::NoticeNotifier+ and serves only the purpose of making the library
# API easier to use.
#
# @since v2.1.0
class NilNoticeNotifier
# @macro see_public_api_method
def notify(_exception, _params = {}, &block); end

# @macro see_public_api_method
def notify_sync(_exception, _params = {}, &block); end

# @macro see_public_api_method
def add_filter(_filter = nil, &_block); end

# @macro see_public_api_method
def delete_filter(_filter_class); end

# @macro see_public_api_method
def build_notice(_exception, _params = {}); end

# @macro see_public_api_method
def close; end

# @macro see_public_api_method
def configured?
false
end

# @macro see_public_api_method
def merge_context(_context); end
end

# NilPerformanceNotifier is a no-op notifier, which mimics
# {Airbrake::PerformanceNotifier} and serves only the purpose of making the
# library API easier to use.
#
# @since v3.2.0
class NilPerformanceNotifier
# @see Airbrake.notify_request
# @see Airbrake.notify_query
def notify(_performance_info); end

# @see Airbrake.notify_request
# @see Airbrake.notify_query
def add_filter(_filter = nil, &_block); end

# @see Airbrake.notify_request
# @see Airbrake.notify_query
def delete_filter(_filter_class); end
end

# NilDeployNotifier is a no-op notifier, which mimics
# {Airbrake::DeployNotifier} and serves only the purpose of making the library
# API easier to use.
#
# @since v3.2.0
class NilDeployNotifier
# @see Airbrake.notify_deploy
def notify(_deploy_info); end
end

@notice_notifier = NilNoticeNotifier.new
@performance_notifier = NilPerformanceNotifier.new
@deploy_notifier = NilDeployNotifier.new
@performance_notifier = PerformanceNotifier.new
@notice_notifier = NoticeNotifier.new
@deploy_notifier = DeployNotifier.new

class << self
# Configures the Airbrake notifier.
Expand All @@ -163,15 +101,11 @@ class << self
def configure
yield config = Airbrake::Config.instance

unless (result = config.validate.value) == :ok
raise Airbrake::Error, result['error']
if (result = config.validate).rejected?
raise Airbrake::Error, result.value['error']
end

Airbrake::Loggable.instance = config.logger

@performance_notifier = PerformanceNotifier.new
@notice_notifier = NoticeNotifier.new
@deploy_notifier = DeployNotifier.new
end

# @return [Boolean] true if the notifier was configured, false otherwise
Expand Down Expand Up @@ -219,7 +153,7 @@ def notify(exception, params = {}, &block)
# @yield [notice] The notice to filter
# @yieldparam [Airbrake::Notice]
# @yieldreturn [void]
# @return [Hash{String=>String}] the reponse from the server
# @return [Airbrake::Promise] the reponse from the server
# @see .notify
def notify_sync(exception, params = {}, &block)
@notice_notifier.notify_sync(exception, params, &block)
Expand Down
9 changes: 9 additions & 0 deletions spec/deploy_notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@
expect(subject.notify({})).to be_an(Airbrake::Promise)
end

context "when config is invalid" do
before { Airbrake::Config.instance.merge(project_id: nil) }

it "returns a rejected promise" do
promise = subject.notify({})
expect(promise).to be_rejected
end
end

context "when environment is configured" do
before { Airbrake::Config.instance.merge(environment: 'fooenv') }

Expand Down
9 changes: 9 additions & 0 deletions spec/notice_notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@
sleep 1
end

context "when config is invalid" do
before { Airbrake::Config.instance.merge(project_id: nil) }

it "returns a rejected promise" do
promise = subject.notify({})
expect(promise).to be_rejected
end
end

context "when a notice is not ignored" do
it "yields the notice" do
expect { |b| subject.notify('ex', &b) }
Expand Down
9 changes: 9 additions & 0 deletions spec/performance_notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,15 @@
).to have_been_made
end

context "when config is invalid" do
before { Airbrake::Config.instance.merge(project_id: nil) }

it "returns a rejected promise" do
promise = subject.notify({})
expect(promise).to be_rejected
end
end

describe "payload grouping" do
let(:flush_period) { 0.5 }

Expand Down

0 comments on commit fe56299

Please sign in to comment.