Skip to content

Commit

Permalink
feat: add api error reporters
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Mar 13, 2018
1 parent bd9be92 commit 579fa39
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 13 deletions.
14 changes: 2 additions & 12 deletions lib/pact_broker/api/resources/base_resource.rb
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions lib/pact_broker/api/resources/error_handler.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions lib/pact_broker/configuration.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
require 'pact_broker/error'

module PactBroker

class ConfigurationError < PactBroker::Error; end

def self.configuration
@@configuration ||= Configuration.default_configuration
end
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions lib/webmachine/convert_request_to_rack_env.rb
Original file line number Diff line number Diff line change
@@ -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
55 changes: 55 additions & 0 deletions spec/lib/pact_broker/api/resources/error_handler_spec.rb
Original file line number Diff line number Diff line change
@@ -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
20 changes: 19 additions & 1 deletion spec/lib/pact_broker/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
51 changes: 51 additions & 0 deletions spec/lib/webmachine/convert_request_to_rack_env_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 579fa39

Please sign in to comment.