refactor: split up error logging, reporting and response generation

lib/pact_broker/api/resources/can_i_deploy_badge.rb

@@ -55,7 +55,7 @@ def moved_temporarily?
             end
           rescue StandardError => e
             # Want to render a badge, even if there's an error
-            badge_service.error_badge_url("error", ErrorHandler.display_message(e, "reference: #{ErrorHandler.generate_error_reference}"))
+            badge_service.error_badge_url("error", ErrorResponseBodyGenerator.display_message(e, "reference: #{PactBroker::Errors.generate_error_reference}"))
           end
         end
 

lib/pact_broker/api/resources/default_base_resource.rb

@@ -1,12 +1,12 @@
 require 'webmachine'
-require 'pact_broker/api/resources/error_handler'
 require 'pact_broker/services'
 require 'pact_broker/api/decorators'
 require 'pact_broker/logging'
 require 'pact_broker/api/pact_broker_urls'
 require 'pact_broker/json'
 require 'pact_broker/pacts/pact_params'
 require 'pact_broker/api/resources/authentication'
+require 'pact_broker/errors'
 
 module PactBroker
   module Api
@@ -96,8 +96,13 @@ def decorator_options options = {}
           { user_options: decorator_context(options) }
         end
 
-        def handle_exception e
-          PactBroker::Api::Resources::ErrorHandler.call(e, request, response)
+        def handle_exception(error)
+          error_reference = PactBroker::Errors.generate_error_reference
+          application_context.error_logger.call(error, error_reference, request)
+          if PactBroker::Errors.reportable_error?(error)
+            PactBroker::Errors.report(error, error_reference, request)
+          end
+          response.body = application_context.error_response_body_generator.call(error, error_reference, request)
         end
 
         def params(options = {})

lib/pact_broker/api/resources/error_response_body_generator.rb

@@ -0,0 +1,41 @@
+require 'pact_broker/configuration'
+
+module PactBroker
+  module Api
+    module Resources
+      class ErrorResponseBodyGenerator
+        include PactBroker::Logging
+
+        # env not needed, just passing in in case PF ever needs it
+        def self.call error, error_reference, env = {}
+          response_body_hash(error, error_reference).to_json
+        end
+
+        def self.display_message(error, obfuscated_message)
+          if PactBroker.configuration.show_backtrace_in_error_response?
+            error.message || obfuscated_message
+          else
+           PactBroker::Errors.reportable_error?(error) ? obfuscated_message : error.message
+          end
+        end
+
+        def self.obfuscated_error_message(error_reference)
+          "An error has occurred. The details have been logged with the reference #{error_reference}"
+        end
+
+        def self.response_body_hash(error, error_reference)
+          response_body = {
+            error: {
+              message: display_message(error, obfuscated_error_message(error_reference)),
+              reference: error_reference
+            }
+          }
+          if PactBroker.configuration.show_backtrace_in_error_response?
+            response_body[:error][:backtrace] = error.backtrace
+          end
+          response_body
+        end
+      end
+    end
+  end
+end

lib/pact_broker/application_context.rb

@@ -3,7 +3,9 @@ class ApplicationContext
     attr_reader :decorator_configuration, :decorator_context_creator, :webhook_execution_configuration_creator,
       :resource_authorizer,
       :before_resource,
-      :after_resource
+      :after_resource,
+      :error_logger,
+      :error_response_body_generator
 
     def initialize(params = {})
       @decorator_configuration = params[:decorator_configuration]
@@ -12,16 +14,23 @@ def initialize(params = {})
       @resource_authorizer = params[:resource_authorizer]
       @before_resource = params[:before_resource]
       @after_resource = params[:after_resource]
+      @error_logger = params[:error_logger]
+      @error_response_body_generator = params[:error_response_body_generator]
     end
 
     def self.default_application_context(overrides = {})
       require 'pact_broker/api/decorators/configuration'
       require 'pact_broker/api/decorators/decorator_context_creator'
       require 'pact_broker/webhooks/execution_configuration_creator'
+      require 'pact_broker/errors/error_logger'
+      require 'pact_broker/api/resources/error_response_body_generator'
+
       defaults = {
         decorator_configuration: PactBroker::Api::Decorators::Configuration.default_configuration,
         decorator_context_creator: PactBroker::Api::Decorators::DecoratorContextCreator,
-        webhook_execution_configuration_creator: PactBroker::Webhooks::ExecutionConfigurationCreator
+        webhook_execution_configuration_creator: PactBroker::Webhooks::ExecutionConfigurationCreator,
+        error_logger: PactBroker::Errors::ErrorLogger,
+        error_response_body_generator: PactBroker::Api::Resources::ErrorResponseBodyGenerator
       }
       ApplicationContext.new(defaults.merge(overrides))
     end

lib/pact_broker/error.rb

@@ -1,6 +1,4 @@
 module PactBroker
-
   class Error < StandardError; end
   class TestError < StandardError; end
-
 end

lib/pact_broker/errors.rb

@@ -0,0 +1,28 @@
+require 'pact_broker/configuration'
+require 'pact_broker/error'
+require 'pact_broker/logging'
+require 'securerandom'
+
+module PactBroker
+  module Errors
+    include PactBroker::Logging
+
+    def self.generate_error_reference
+      SecureRandom.urlsafe_base64.gsub(/[^a-z]/i, '')[0,10]
+    end
+
+    def self.reportable_error?(error)
+      !error.is_a?(PactBroker::Error) && !error.is_a?(JSON::JSONError)
+    end
+
+    def self.report error, error_reference, request
+      PactBroker.configuration.api_error_reporters.each do | error_notifier |
+        begin
+          error_notifier.call(error, env: request.env, error_reference: error_reference)
+        rescue StandardError => e
+          log_error(e, "Error executing api_error_reporter")
+        end
+      end
+    end
+  end
+end

lib/pact_broker/errors/error_logger.rb

@@ -0,0 +1,25 @@
+require 'pact_broker/configuration'
+require 'pact_broker/logging'
+
+module PactBroker
+  module Errors
+    class ErrorLogger
+      include PactBroker::Logging
+
+      # don't need the env, just in case PF needs it
+      def self.call(error, error_reference, env = {})
+        if log_as_warning?(error)
+          logger.warn("Error reference #{error_reference}", error)
+        elsif PactBroker::Errors.reportable_error?(error)
+          log_error(error, "Error reference #{error_reference}")
+        else
+          logger.info("Error reference #{error_reference}", error)
+        end
+      end
+
+      def self.log_as_warning?(error)
+        PactBroker.configuration.warning_error_classes.any? { |clazz| error.is_a?(clazz) || error.cause&.is_a?(clazz) }
+      end
+    end
+  end
+end

spec/lib/pact_broker/api/resources/can_i_deploy_badge_spec.rb

@@ -15,7 +15,7 @@ module Resources
           allow(pacticipant_service).to receive(:find_pacticipant_by_name).and_return(pacticipant)
           allow(version_service).to receive(:find_by_pacticipant_name_and_latest_tag).and_return(version)
           allow(PactBroker.configuration).to receive(:show_backtrace_in_error_response?).and_return(false)
-          allow(ErrorHandler).to receive(:generate_error_reference).and_return("abcd")
+          allow(PactBroker::Errors).to receive(:generate_error_reference).and_return("abcd")
         end
 
         let(:pacticipant_service) { class_double("PactBroker::Pacticipant::Service").as_stubbed_const }