feat: add api error reporters

Author: bethesque

lib/pact_broker/api/resources/base_resource.rb

@@ -1,4 +1,5 @@
 require 'webmachine'
+require 'pact_broker/api/resources/error_handler'
 require 'pact_broker/services'
 require 'pact_broker/api/decorators'
 require 'pact_broker/logging'
@@ -15,17 +16,6 @@ module Resources
 
       class InvalidJsonError < StandardError ; end
 
-      class ErrorHandler
-
-        include PactBroker::Logging
-
-        def self.handle_exception e, response
-          logger.error e
-          logger.error e.backtrace
-          response.body = {:message => e.message, :backtrace => e.backtrace }.to_json
-        end
-      end
-
       class BaseResource < Webmachine::Resource
 
         include PactBroker::Services
@@ -90,7 +80,7 @@ def decorator_context options = {}
         end
 
         def handle_exception e
-          PactBroker::Api::Resources::ErrorHandler.handle_exception(e, response)
+          PactBroker::Api::Resources::ErrorHandler.call(e, request, response)
         end
 
         def params

lib/pact_broker/api/resources/error_handler.rb

@@ -0,0 +1,30 @@
+require 'webmachine/convert_request_to_rack_env'
+require 'pact_broker/configuration'
+
+module PactBroker
+  module Api
+    module Resources
+      class ErrorHandler
+
+        include PactBroker::Logging
+
+        def self.call e, request, response
+          logger.error e
+          logger.error e.backtrace
+          response.body = {:message => e.message, :backtrace => e.backtrace }.to_json
+          report e, request
+        end
+
+        def self.report e, request
+          PactBroker.configuration.api_error_reporters.each do | error_notifier |
+            begin
+              error_notifier.call(e, env: Webmachine::ConvertRequestToRackEnv.call(request))
+            rescue StandardError => e
+              log_error(e, "Error executing api_error_reporter")
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/pact_broker/configuration.rb

@@ -1,5 +1,9 @@
+require 'pact_broker/error'
+
 module PactBroker
 
+  class ConfigurationError < PactBroker::Error; end
+
   def self.configuration
     @@configuration ||= Configuration.default_configuration
   end
@@ -30,13 +34,15 @@ class Configuration
     attr_accessor :enable_public_badge_access, :shields_io_base_url
     attr_accessor :webhook_retry_schedule
     attr_accessor :disable_ssl_verification
+    attr_reader :api_error_reporters
     attr_writer :logger
 
     def initialize
       @before_resource_hook = ->(resource){}
       @after_resource_hook = ->(resource){}
       @authenticate_with_basic_auth = nil
       @authorize = nil
+      @api_error_reporters = []
     end
 
     def logger
@@ -121,6 +127,16 @@ def after_resource &block
       end
     end
 
+    def add_api_error_reporter &block
+      if block_given?
+        unless block.arity == 2
+          raise ConfigurationError.new("api_error_notfifier block must accept two arguments, 'error' and 'options'")
+        end
+        @api_error_reporters << block
+        nil
+      end
+    end
+
     def enable_badge_resources= enable_badge_resources
       puts "Pact Broker configuration property `enable_badge_resources` is deprecated. Please use `enable_public_badge_access`"
       self.enable_public_badge_access = enable_badge_resources

lib/webmachine/convert_request_to_rack_env.rb

@@ -0,0 +1,29 @@
+module Webmachine
+  class ConvertRequestToRackEnv
+    def self.call(request)
+      env = {
+        'REQUEST_METHOD' => request.method.upcase,
+        'CONTENT_TYPE' => request.headers['Content-Type'],
+        'PATH_INFO' => request.uri.path,
+        'QUERY_STRING' => request.uri.query || "",
+        'SERVER_NAME' => request.uri.host,
+        'SERVER_PORT' => request.uri.port.to_s,
+        'SCRIPT_NAME' => '',
+        'rack.url_scheme' => request.uri.scheme,
+        'rack.input' => request.body.to_io ? StringIO.new(request.body.to_s) : nil
+      }
+      http_headers = request.headers.each do | key, value |
+        env[convert_http_header_name_to_rack_header_name(key)] = value
+      end
+      env
+    end
+
+    def self.convert_http_header_name_to_rack_header_name(http_header_name)
+      if http_header_name.downcase == 'content-type' || http_header_name.downcase ==  'content-length'
+        http_header_name.upcase.gsub('-', '_')
+      else
+        "HTTP_" + http_header_name.upcase.gsub('-', '_')
+      end
+    end
+  end
+end

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

@@ -0,0 +1,55 @@
+require 'pact_broker/api/resources/error_handler'
+
+module PactBroker
+  module Api
+    module Resources
+      describe ErrorHandler do
+        describe "call" do
+          let(:error) { PactBroker::Error.new('test error') }
+          let(:thing) { double('thing', call: nil, another_call: nil) }
+          let(:options) { { env: env } }
+          let(:request) { double('request' ) }
+          let(:response) { double('response', :body= => nil) }
+          let(:env) { double('env') }
+
+          subject { ErrorHandler.call(error, request, response) }
+
+          before do
+            allow(Webmachine::ConvertRequestToRackEnv).to receive(:call).and_return(env)
+            PactBroker.configuration.add_api_error_reporter do | error, options |
+              thing.call(error, options)
+            end
+
+            PactBroker.configuration.add_api_error_reporter do | error, options |
+              thing.another_call(error, options)
+            end
+          end
+
+          it "invokes the api error reporters" do
+            expect(thing).to receive(:call).with(error, options)
+            expect(thing).to receive(:another_call).with(error, options)
+            subject
+          end
+
+          context "when the error reporter raises an error itself" do
+            class TestError < StandardError; end
+
+            before do
+              expect(thing).to receive(:call).and_raise(TestError.new)
+            end
+
+            it "logs the error" do
+              expect(PactBroker.logger).to receive(:error).at_least(1).times
+              subject
+            end
+
+            it "does not propagate the error" do
+              expect(thing).to receive(:another_call)
+              subject
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/configuration_spec.rb

@@ -40,7 +40,7 @@ module PactBroker
       end
 
       describe "load_from_database!" do
-        let(:configuration) { PactBroker::Configuration.new}
+        let(:configuration) { PactBroker::Configuration.new }
 
         before do
           PactBroker::Config::Setting.create(name: 'use_case_sensitive_resource_names', type: 'string', value: 'foo')
@@ -51,6 +51,24 @@ module PactBroker
           expect(configuration.use_case_sensitive_resource_names).to eq "foo"
         end
       end
+
+      describe "add_api_error_reporter" do
+        let(:configuration) { PactBroker::Configuration.new }
+        let(:block) { Proc.new{ | error, options | } }
+
+        it "should add the error notifier " do
+          configuration.add_api_error_reporter(&block)
+          expect(configuration.api_error_reporters.first).to eq block
+        end
+
+        context "with a proc with the wrong number of arguments" do
+          let(:block) { Proc.new{ | error | } }
+
+          it "raises an error" do
+            expect { configuration.add_api_error_reporter(&block) }.to raise_error PactBroker::ConfigurationError
+          end
+        end
+      end
     end
   end
 end

spec/lib/webmachine/convert_request_to_rack_env_spec.rb

@@ -0,0 +1,51 @@
+require 'webmachine/convert_request_to_rack_env'
+require 'webmachine/request'
+
+module Webmachine
+  describe ConvertRequestToRackEnv do
+
+    let(:rack_env) do
+      {
+        "rack.input"=>StringIO.new('foo'),
+        "REQUEST_METHOD"=>"POST",
+        "SERVER_NAME"=>"example.org",
+        "SERVER_PORT"=>"80",
+        "QUERY_STRING"=>"",
+        "PATH_INFO"=>"/foo",
+        "rack.url_scheme"=>"http",
+        "SCRIPT_NAME"=>"",
+        "CONTENT_LENGTH"=>"0",
+        "HTTP_HOST"=>"example.org",
+        "CONTENT_TYPE"=>"application/x-www-form-urlencoded",
+      }
+    end
+
+    let(:headers) do
+      Webmachine::Headers.from_cgi(rack_env)
+    end
+
+    let(:rack_req) { ::Rack::Request.new(rack_env) }
+    let(:webmachine_request) do
+      Webmachine::Request.new(rack_req.request_method,
+                              rack_req.url,
+                              headers,
+                              Webmachine::Adapters::Rack::RequestBody.new(rack_req),
+                              nil,
+                              nil
+                             )
+    end
+
+    subject { ConvertRequestToRackEnv.call(webmachine_request) }
+
+    describe ".call" do
+      it "" do
+        expected_env = rack_env.dup
+        expected_env.delete('rack.input')
+        actual_env = subject
+        actual_rack_input = actual_env.delete('rack.input')
+        expect(subject).to eq expected_env
+        expect(actual_rack_input.string).to eq 'foo'
+      end
+    end
+  end
+end