Skip to content

Commit

Permalink
Merge pull request #520 from airbrake/517-close-fix
Browse files Browse the repository at this point in the history
airbrake-ruby: don't spawn notifiers on #close calls
  • Loading branch information
kyrylo authored Dec 12, 2019
2 parents 5c8e189 + 690b0b1 commit 60b67bb
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 39 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Airbrake Ruby Changelog
* Added `Airbrake.notify_request_sync`, `Airbrake.notify_query_sync` &
`Airbrake.notify_performance_sync`
([#519](https://github.com/airbrake/airbrake-ruby/pull/519))
* Fixed bug where `Airbrake.close` would spawn notifiers (if they didn't exist)
just to close them immediately
([#520](https://github.com/airbrake/airbrake-ruby/pull/520))

### [v4.9.0][v4.9.0] (December 9, 2019)

Expand Down
15 changes: 11 additions & 4 deletions lib/airbrake-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def deploy_notifier
# @return [Boolean] true if the notifier was configured, false otherwise
# @since v2.3.0
def configured?
notice_notifier.configured?
@notice_notifier && @notice_notifier.configured?
end

# Sends an exception to Airbrake asynchronously.
Expand Down Expand Up @@ -255,10 +255,17 @@ def build_notice(exception, params = {})
# Airbrake.notify('App crashed!') #=> raises Airbrake::Error
#
# @return [void]
# rubocop:disable Style/GuardClause, Style/IfUnlessModifier
def close
notice_notifier.close
performance_notifier.close
if defined?(@notice_notifier) && @notice_notifier
@notice_notifier.close
end

if defined?(@performance_notifier) && @performance_notifier
@performance_notifier.close
end
end
# rubocop:enable Style/GuardClause, Style/IfUnlessModifier

# Pings the Airbrake Deploy API endpoint about the occurred deploy.
#
Expand Down Expand Up @@ -551,7 +558,7 @@ def delete_performance_filter(filter_class)
# @return [void]
# @since v4.2.2
def reset
close if notice_notifier && configured?
close

self.performance_notifier = PerformanceNotifier.new
self.notice_notifier = NoticeNotifier.new
Expand Down
66 changes: 31 additions & 35 deletions spec/airbrake_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,28 +51,6 @@
expect(described_class).to be_configured
end

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

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

context "when a notifier wasn't configured" do
before do
expect(described_class).to receive(:configured?).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

context "when called multiple times" do
it "doesn't overwrite performance notifier" do
described_class.configure {}
Expand Down Expand Up @@ -182,19 +160,6 @@
end
end

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

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

describe "#notify_request" do
context "when :stash key is not provided" do
it "doesn't add anything to the stash of the request" do
Expand Down Expand Up @@ -415,4 +380,35 @@
expect(described_class.deploy_notifier).to be_an(Airbrake::DeployNotifier)
end
end

describe ".close" do
after { Airbrake.reset }

context "when notice_notifier is defined" do
it "gets closed" do
expect(Airbrake.notice_notifier).to receive(:close)
end
end

context "when notice_notifier is undefined" do
it "doesn't get closed (because it wasn't initialized)" do
Airbrake.instance_variable_set(:@notice_notifier, nil)
expect_any_instance_of(Airbrake::NoticeNotifier).not_to receive(:close)
end
end

context "when performance_notifier is defined" do
it "gets closed" do
expect(Airbrake.performance_notifier).to receive(:close)
end
end

context "when perforance_notifier is undefined" do
it "doesn't get closed (because it wasn't initialized)" do
Airbrake.instance_variable_set(:@performance_notifier, nil)
expect_any_instance_of(Airbrake::PerformanceNotifier)
.not_to receive(:close)
end
end
end
end

0 comments on commit 60b67bb

Please sign in to comment.