From a627629f1032984848648e0d02a0abf123c2250c Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Thu, 13 Apr 2017 19:14:24 +0300 Subject: [PATCH] airbrake-ruby,notifier: use the Null Object pattern for notifiers Fixes https://github.com/airbrake/airbrake/issues/713 (undefined method `build_notice' for nil:NilClass) It becomes really tedious to keep track of whether a notifier is configured or not. Since we introduced `Airbrake.[]` to retrieve notifiers, all integrations have to add an additional check for it: ```rb if notifier = Airbrake[:default] notifier.notify(...) end ``` With the current change this check is not needed anymore. --- CHANGELOG.md | 4 ++++ README.md | 3 +++ lib/airbrake-ruby.rb | 17 +++++++++-------- lib/airbrake-ruby/notifier.rb | 19 +++++++++++++++++++ spec/airbrake_spec.rb | 10 ++++++++-- 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eda99d2..05758f29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ Airbrake Ruby Changelog ### master +* Return `Airbrake::NilNotifier` when no notifiers are configured and + `Airbrake.[]` is called + ([#191](https://github.com/airbrake/airbrake-ruby/pull/191)) + ### [v2.0.0][v2.0.0] (March 21, 2017) * **IMPORTANT:** Removed the `component/action` API deprecated diff --git a/README.md b/README.md index 9eca833f..06f87a26 100644 --- a/README.md +++ b/README.md @@ -409,6 +409,9 @@ Retrieves a configured notifier. Airbrake[:my_notifier] #=> Airbrake::Notifier ``` +If the notifier is not configured, returns an instance of +`Airbrake::NilNotifier` (a no-op version of `Airbrake::Notifier`). + #### Airbrake.notify Sends an exception to Airbrake asynchronously. diff --git a/lib/airbrake-ruby.rb b/lib/airbrake-ruby.rb index 42f8d828..75c653c9 100644 --- a/lib/airbrake-ruby.rb +++ b/lib/airbrake-ruby.rb @@ -73,8 +73,9 @@ module Airbrake ## # A Hash that holds all notifiers. The keys of the Hash are notifier - # names, the values are Airbrake::Notifier instances. - @notifiers = {} + # names, the values are Airbrake::Notifier instances. If a notifier is not + # assigned to the hash, then it returns a null object (NilNotifier). + @notifiers = Hash.new(NilNotifier.new) class << self ## @@ -146,7 +147,7 @@ def configure(notifier_name = :default) # @return [Airbrake::Promise] # @see .notify_sync def notify(exception, params = {}) - @notifiers[:default] && @notifiers[:default].notify(exception, params) + @notifiers[:default].notify(exception, params) end ## @@ -159,7 +160,7 @@ def notify(exception, params = {}) # @return [Hash{String=>String}] the reponse from the server # @see .notify def notify_sync(exception, params = {}) - @notifiers[:default] && @notifiers[:default].notify_sync(exception, params) + @notifiers[:default].notify_sync(exception, params) end ## @@ -189,7 +190,7 @@ def notify_sync(exception, params = {}) # @return [void] # @note Once a filter was added, there's no way to delete it def add_filter(filter = nil, &block) - @notifiers[:default] && @notifiers[:default].add_filter(filter, &block) + @notifiers[:default].add_filter(filter, &block) end ## @@ -208,7 +209,7 @@ def add_filter(filter = nil, &block) # @return [Airbrake::Notice] the notice built with help of the given # arguments def build_notice(exception, params = {}) - @notifiers[:default] && @notifiers[:default].build_notice(exception, params) + @notifiers[:default].build_notice(exception, params) end ## @@ -222,7 +223,7 @@ def build_notice(exception, params = {}) # # @return [void] def close - @notifiers[:default] && @notifiers[:default].close + @notifiers[:default].close end ## @@ -237,7 +238,7 @@ def close # @option deploy_params [Symbol] :version # @return [void] def create_deploy(deploy_params) - @notifiers[:default] && @notifiers[:default].create_deploy(deploy_params) + @notifiers[:default].create_deploy(deploy_params) end end end diff --git a/lib/airbrake-ruby/notifier.rb b/lib/airbrake-ruby/notifier.rb index a57e6179..11b4f63e 100644 --- a/lib/airbrake-ruby/notifier.rb +++ b/lib/airbrake-ruby/notifier.rb @@ -159,4 +159,23 @@ def add_filters_for_config_keys add_filter(Filters::KeysWhitelist.new(@config.logger, *@config.whitelist_keys)) end end + + ## + # NilNotifier is a no-op notifier, which mimics +Airbrake::Notifier+ and + # serves only for the purpose of making the library API easier to use. + # + # @since 2.1.0 + class NilNotifier + def notify(_exception, _params = {}); end + + def notify_sync(_exception, _params); end + + def add_filter(_filter = nil, &_block); end + + def build_notice(_exception, _params); end + + def close; end + + def create_deploy(_deploy_params); end + end end diff --git a/spec/airbrake_spec.rb b/spec/airbrake_spec.rb index 308111a0..b74ff753 100644 --- a/spec/airbrake_spec.rb +++ b/spec/airbrake_spec.rb @@ -17,12 +17,18 @@ end after do - described_class.instance_variable_set(:@notifiers, {}) + described_class.instance_variable_set( + :@notifiers, + Hash.new(Airbrake::NilNotifier.new) + ) end shared_examples 'non-configured notifier handling' do |method| it "returns nil if there is no configured notifier when using #{method}" do - described_class.instance_variable_set(:@notifiers, {}) + described_class.instance_variable_set( + :@notifiers, + Hash.new(Airbrake::NilNotifier.new) + ) expect(described_class.__send__(method, 'bingo')).to be_nil end end