Skip to content

Commit

Permalink
feat: log foreign key constraint errors as warn as 99% of the time th…
Browse files Browse the repository at this point in the history
…ey are transitory and unreproducible and should not cause alarms to be raised
  • Loading branch information
bethesque committed Jun 19, 2020
1 parent 1e097b3 commit 71fd027
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
13 changes: 10 additions & 3 deletions lib/pact_broker/api/resources/error_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions lib/pact_broker/db/log_quietener.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."
Expand Down
10 changes: 10 additions & 0 deletions spec/lib/pact_broker/db/log_quietener_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 71fd027

Please sign in to comment.