Skip to content

Commit

Permalink
airbrake-ruby: make sure optional filters are loaded
Browse files Browse the repository at this point in the history
Fixes #460 (blacklist_keys config doesn't work)

The problem was that we never reconfigured already instantiated notifiers. We
always spawn notifiers as soon as the library is required but they get spawned
without some of default filters that are supposed to work given certain options
such as `blacklist_keys`.

The new `#reset` method respawns all notifiers on `#configure`. We have to
respawn so the new notifiers pickup the config. We need to spawn without the
user config so all the methods are working (e.g. notify returns a rejected
promise and such).
  • Loading branch information
kyrylo committed Mar 26, 2019
1 parent 0cd8c41 commit 9b55860
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 5 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

* Fixed `blacklist_keys/whitelist_keys` options not working
([#461](https://github.com/airbrake/airbrake-ruby/pull/461))

### [v4.2.1][v4.2.1] (March 14, 2019)

* Stopped raising `Airbrake::Error` when configuring Airbrake without a project
Expand Down
18 changes: 14 additions & 4 deletions lib/airbrake-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ module Airbrake
# @!macro see_public_api_method
# @see Airbrake.$0

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

class << self
# Configures the Airbrake notifier.
#
Expand All @@ -100,6 +96,7 @@ class << self
def configure
yield config = Airbrake::Config.instance
Airbrake::Loggable.instance = config.logger
reset
end

# @return [Boolean] true if the notifier was configured, false otherwise
Expand Down Expand Up @@ -436,5 +433,18 @@ def add_performance_filter(filter = nil, &block)
def delete_performance_filter(filter_class)
@performance_notifier.delete_filter(filter_class)
end

# Resets all notifiers, including its filters
# @return [void]
# @since v4.2.2
def reset
close if @notice_notifier && configured?

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

Airbrake.reset
53 changes: 52 additions & 1 deletion spec/airbrake_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
RSpec.describe Airbrake do
before { Airbrake::Config.instance = Airbrake::Config.new }
before do
Airbrake::Config.instance = Airbrake::Config.new
described_class.reset
end

after { described_class.reset }

describe ".configure" do
it "yields the config" do
Expand All @@ -22,5 +27,51 @@

expect(Airbrake::Loggable.instance).to eql(logger)
end

it "makes Airbrake configured" do
expect(described_class).not_to be_configured

described_class.configure do |c|
c.project_id = 1
c.project_key = '2'
end

expect(described_class).to be_configured
end

context "when a notifier was configured" do
before do
expect(described_class).to receive(:configured?).twice.and_return(true)
end

it "closes previously configured notice notifier" do
expect(described_class).to receive(:close).twice
described_class.configure {}
end
end

context "when a notifier wasn't configured" do
before do
expect(described_class).to receive(:configured?).twice.and_return(false)
end

it "doesn't close previously configured notice notifier" do
expect(described_class).not_to receive(:close)
described_class.configure {}
end
end
end

describe "#reset" do
context "when Airbrake was previously configured" do
before do
expect(described_class).to receive(:configured?).twice.and_return(true)
end

it "closes notice notifier" do
expect(described_class).to receive(:close).twice
subject.reset
end
end
end
end

0 comments on commit 9b55860

Please sign in to comment.