From 817dc77be8c42a0eef8a854701396217783e02d4 Mon Sep 17 00:00:00 2001
From: Thorsten Ball <mrnugget@gmail.com>
Date: Fri, 20 Jul 2018 08:57:05 +0200
Subject: [PATCH] Do not report SignalExceptions in at_exit handler

The previous behaviour resulted in applications reporting errors even
though they were gracefully and without any problems shut down. Example:
the puma webserver relies on `SIGTERM` to gracefully shut down its
workers and Bugsnag reported errors when it was normally shut down.

For more information on puma's behaviour, see here:

    https://github.com/puma/puma/issues/1438

With the `at_exit` handler introduced in #397, Bugsnag would catch
`SignalExceptions` on exit and report them as error, even though they
are none.

This PR changes the existing behaviour to add `SignalException` as a
special case in which the exception should not be passed to
`Bugsnag.notify`.

Due to the Ruby exiting when a `SignalException` is raised I had to
refactor the code to make the actual handling of the exit-exceptions
testable without the test suite quitting.
---
 lib/bugsnag.rb       | 20 +++++++------
 spec/bugsnag_spec.rb | 67 ++++++++++++++++++++++++++++++--------------
 2 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb
index 00985b9eb..84af47e98 100644
--- a/lib/bugsnag.rb
+++ b/lib/bugsnag.rb
@@ -118,15 +118,17 @@ def notify(exception, auto_notify=false, &block)
     def register_at_exit
       return if at_exit_handler_installed?
       @exit_handler_added = true
-      at_exit do
-        if $!
-          Bugsnag.notify($!, true) do |report|
-            report.severity = 'error'
-            report.severity_reason = {
-              :type => Bugsnag::Report::UNHANDLED_EXCEPTION
-            }
-          end
-        end
+      at_exit { $! && at_exit_handler($!) }
+    end
+
+    def at_exit_handler(exception)
+      return if exception.is_a?(SignalException)
+
+      Bugsnag.notify(exception, true) do |report|
+        report.severity = 'error'
+        report.severity_reason = {
+          :type => Bugsnag::Report::UNHANDLED_EXCEPTION
+        }
       end
     end
 
diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb
index 8e8e8a5b5..3b2b03e9a 100644
--- a/spec/bugsnag_spec.rb
+++ b/spec/bugsnag_spec.rb
@@ -41,33 +41,58 @@
       Bugsnag.register_at_exit
     end
 
+    describe 'at_exit_handler' do
+      context 'with a non-signal exception' do
+        it 'reports the exception' do
+          exception = BugsnagTestException.new('Oh no')
+
+          report_mock = double('report')
+          expect(report_mock).to receive(:severity=).with('error')
+          expect(report_mock).to receive(:severity_reason=).with({
+            :type => Bugsnag::Report::UNHANDLED_EXCEPTION
+          })
+
+          expect(Bugsnag).to receive(:notify).with(exception, true).and_yield(report_mock)
+
+          Bugsnag.at_exit_handler(exception)
+        end
+      end
+
+      context 'with a signal exception' do
+        it 'does not report the exception' do
+          exception = SignalException.new('SIGTERM')
+
+          expect(Bugsnag).to_not receive(:notify).with(exception, true)
+
+          Bugsnag.at_exit_handler(exception)
+        end
+      end
+    end
+
     context 'with aliased at_exit' do
-      before do
-        module Kernel
-          alias_method :old_at_exit, :at_exit
-          def at_exit
-            begin
-              raise BugsnagTestException.new("Oh no")
-            rescue
-              yield
+      context 'and a non-signal exception' do
+        before do
+          module Kernel
+            alias_method :old_at_exit, :at_exit
+            def at_exit
+              begin
+                raise BugsnagTestException.new("Oh no")
+              rescue
+                yield
+              end
             end
           end
         end
-      end
 
-      it "sends an exception when at_exit is called" do
-        report_mock = double('report')
-        expect(report_mock).to receive(:severity=).with('error')
-        expect(report_mock).to receive(:severity_reason=).with({
-          :type => Bugsnag::Report::UNHANDLED_EXCEPTION
-        })
-        expect(Bugsnag).to receive(:notify).with(kind_of(BugsnagTestException), true).and_yield(report_mock)
-        Bugsnag.register_at_exit
-      end
+        it "calls the at_exit_handler" do
+          expect(Bugsnag).to receive(:at_exit_handler)
+          Bugsnag.register_at_exit
+        end
 
-      after do
-        module Kernel
-          alias_method :at_exit, :old_at_exit
+        after do
+          module Kernel
+            alias_method :at_exit, :old_at_exit
+          end
         end
       end
     end