diff --git a/lib/pact_broker/api/resources/base_resource.rb b/lib/pact_broker/api/resources/base_resource.rb index e533b1c51..65e3c5635 100644 --- a/lib/pact_broker/api/resources/base_resource.rb +++ b/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 diff --git a/lib/pact_broker/api/resources/error_handler.rb b/lib/pact_broker/api/resources/error_handler.rb new file mode 100644 index 000000000..6c8a972a3 --- /dev/null +++ b/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 diff --git a/lib/pact_broker/configuration.rb b/lib/pact_broker/configuration.rb index d11cc17f9..fe4afb086 100644 --- a/lib/pact_broker/configuration.rb +++ b/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,6 +34,7 @@ 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 @@ -37,6 +42,7 @@ def initialize @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 diff --git a/lib/webmachine/convert_request_to_rack_env.rb b/lib/webmachine/convert_request_to_rack_env.rb new file mode 100644 index 000000000..280dfb903 --- /dev/null +++ b/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 diff --git a/spec/lib/pact_broker/api/resources/error_handler_spec.rb b/spec/lib/pact_broker/api/resources/error_handler_spec.rb new file mode 100644 index 000000000..c00c7562a --- /dev/null +++ b/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 diff --git a/spec/lib/pact_broker/configuration_spec.rb b/spec/lib/pact_broker/configuration_spec.rb index fb1505f15..e56952c8e 100644 --- a/spec/lib/pact_broker/configuration_spec.rb +++ b/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 diff --git a/spec/lib/webmachine/convert_request_to_rack_env_spec.rb b/spec/lib/webmachine/convert_request_to_rack_env_spec.rb new file mode 100644 index 000000000..b94b989c2 --- /dev/null +++ b/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