From 71fd02705bcc5cb330b856c0ebdba0b728a72d35 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 19 Jun 2020 11:41:32 +1000
Subject: [PATCH] feat: log foreign key constraint errors as warn as 99% of the
 time they are transitory and unreproducible and should not cause alarms to be
 raised

---
 lib/pact_broker/api/resources/error_handler.rb | 13 ++++++++++---
 lib/pact_broker/db/log_quietener.rb            |  7 +++++++
 spec/lib/pact_broker/db/log_quietener_spec.rb  | 10 ++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/lib/pact_broker/api/resources/error_handler.rb b/lib/pact_broker/api/resources/error_handler.rb
index 52924cdf8..966749874 100644
--- a/lib/pact_broker/api/resources/error_handler.rb
+++ b/lib/pact_broker/api/resources/error_handler.rb
@@ -8,13 +8,16 @@ class ErrorHandler
 
         include PactBroker::Logging
 
+        WARNING_ERROR_CLASSES = [Sequel::ForeignKeyConstraintViolation]
+
         def self.call e, request, response
           error_reference = generate_error_reference
-          if reportable?(e)
+          if log_as_warning?(e)
+            logger.warn("Error reference #{error_reference}", e)
+          elsif reportable?(e)
             log_error(e, "Error reference #{error_reference}")
             report(e, error_reference, request)
-          else
-            logger.info "Error reference #{error_reference} - #{e.class} #{e.message}\n#{e.backtrace.join("\n")}"
+            logger.info("Error reference #{error_reference}", e)
           end
           response.body = response_body_hash(e, error_reference).to_json
         end
@@ -27,6 +30,10 @@ def self.reportable?(e)
           !e.is_a?(PactBroker::Error) && !e.is_a?(JSON::GeneratorError)
         end
 
+        def self.log_as_warning?(e)
+          WARNING_ERROR_CLASSES.any? { |clazz| e.is_a?(clazz) }
+        end
+
         def self.display_message(e, error_reference)
           if PactBroker.configuration.show_backtrace_in_error_response?
             e.message || obfuscated_error_message(error_reference)
diff --git a/lib/pact_broker/db/log_quietener.rb b/lib/pact_broker/db/log_quietener.rb
index 67cb00b68..2ba3ef334 100644
--- a/lib/pact_broker/db/log_quietener.rb
+++ b/lib/pact_broker/db/log_quietener.rb
@@ -16,6 +16,8 @@ def info *args
       def error *args
         if error_is_about_table_not_existing?(args)
           __getobj__().debug(*reassure_people_that_this_is_expected(args))
+        elsif foreign_key_error?(args)
+          __getobj__().warn(*args)
         else
           __getobj__().error(*args)
         end
@@ -28,6 +30,11 @@ def error_is_about_table_not_existing?(args)
             args.first.include?("no such view"))
       end
 
+      # Foreign key exceptions are almost always transitory and unreproducible by this stage
+      def foreign_key_error?(args)
+        args.first.is_a?(String) && args.first.downcase.include?("foreign key")
+      end
+
       def reassure_people_that_this_is_expected(args)
         message = args.shift
         message = message + " Don't panic. This happens when Sequel doesn't know if a table/view exists or not."
diff --git a/spec/lib/pact_broker/db/log_quietener_spec.rb b/spec/lib/pact_broker/db/log_quietener_spec.rb
index 6b6ddbd87..89700cde5 100644
--- a/spec/lib/pact_broker/db/log_quietener_spec.rb
+++ b/spec/lib/pact_broker/db/log_quietener_spec.rb
@@ -23,6 +23,16 @@ module DB
           end
         end
 
+        context "when the error is a foreign key constraint violation" do
+          before do
+            subject.error("SQLite3::ConstraintException: FOREIGN KEY constraint failed: delete from pacticipants where id = 1")
+          end
+
+          it "logs the message at warn level" do
+            expect(logs.string).to include "WARN -- :"
+          end
+        end
+
         context "when the error is NOT for a table or view that does not exist" do
           before do
             subject.error("foo bar")