diff --git a/lib/pact_broker/api/resources/error_handler.rb b/lib/pact_broker/api/resources/error_handler.rb index 290d9dd9d..17267f12d 100644 --- a/lib/pact_broker/api/resources/error_handler.rb +++ b/lib/pact_broker/api/resources/error_handler.rb @@ -1,5 +1,6 @@ require 'webmachine/convert_request_to_rack_env' require 'pact_broker/configuration' +require 'securerandom' module PactBroker module Api @@ -10,24 +11,46 @@ class ErrorHandler include PactBroker::Logging def self.call e, request, response - logger.error e + error_reference = SecureRandom.urlsafe_base64.gsub(/[^a-z]/i, '')[0,10] + logger.error "#{e.message} - error reference #{error_reference}" logger.error e.backtrace - response_body = { error: { :message => e.message } } - if PactBroker.configuration.show_backtrace_in_error_response? - response_body[:error][:backtrace] = e.backtrace - end - response.body = response_body.to_json - report(e, request) if reportable?(e) + response.body = response_body_hash(e, error_reference).to_json + report(e, error_reference, request) if reportable?(e) end def self.reportable? e !e.is_a?(PactBroker::Error) end - def self.report e, request + def self.display_message(e, error_reference) + if PactBroker.configuration.show_backtrace_in_error_response? + e.message + else + reportable?(e) ? obfuscated_error_message(error_reference) : e.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 e, error_reference + response_body = { + error: { + message: display_message(e, error_reference), + reference: error_reference + } + } + if PactBroker.configuration.show_backtrace_in_error_response? + response_body[:error][:backtrace] = e.backtrace + end + response_body + end + + def self.report e, error_reference, request PactBroker.configuration.api_error_reporters.each do | error_notifier | begin - error_notifier.call(e, env: Webmachine::ConvertRequestToRackEnv.call(request)) + error_notifier.call(e, env: Webmachine::ConvertRequestToRackEnv.call(request), error_reference: error_reference) rescue StandardError => e log_error(e, "Error executing api_error_reporter") 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 index c47f2c824..4522c91be 100644 --- a/spec/lib/pact_broker/api/resources/error_handler_spec.rb +++ b/spec/lib/pact_broker/api/resources/error_handler_spec.rb @@ -8,12 +8,13 @@ module Resources before do allow(ErrorHandler).to receive(:logger).and_return(logger) + allow(SecureRandom).to receive(:urlsafe_base64).and_return("bYWfn-+yWPlf") end let(:logger) { double('logger').as_null_object } let(:error) { StandardError.new('test error') } let(:thing) { double('thing', call: nil, another_call: nil) } - let(:options) { { env: env } } + let(:options) { { env: env, error_reference: "bYWfnyWPlf" } } let(:request) { double('request' ) } let(:response) { double('response', :body= => nil) } let(:env) { double('env') } @@ -37,33 +38,59 @@ module Resources subject end - context "when the error is a PactBroker::Error or subclass" do - let(:error) { Class.new(PactBroker::Error).new('test error') } - - it "does not invoke the api error reporters" do - expect(thing).to_not receive(:call).with(error, options) - subject - end - end - - it "creates a json error response body" do + it "includes an error reference" do expect(response).to receive(:body=) do | body | - expect(JSON.parse(body)['error']).to include 'message' => 'test error' + expect(JSON.parse(body)['error']).to include 'reference' => "bYWfnyWPlf" end subject end - context "when show_backtrace_in_error_response? is true" do before do allow(PactBroker.configuration).to receive(:show_backtrace_in_error_response?).and_return(true) end - it "includes the backtrace in the error response" do - expect(response).to receive(:body=) do | body | - expect(body).to include("backtrace") + context "when the error is a PactBroker::Error or subclass" do + let(:error) { Class.new(PactBroker::Error).new('test error') } + + it "does not invoke the api error reporters" do + expect(thing).to_not receive(:call).with(error, options) + subject + end + + it "uses the error message as the message" do + expect(response).to receive(:body=) do | body | + expect(JSON.parse(body)['error']).to include 'message' => "test error" + end + subject + end + + it "includes the backtrace in the error response" do + expect(response).to receive(:body=) do | body | + expect(body).to include("backtrace") + end + subject + end + end + context "when the error is not a PactBroker::Error or subclass" do + it "invokes the api error reporters" do + expect(thing).to receive(:call).with(error, options) + subject + end + + it "uses the error message as the message" do + expect(response).to receive(:body=) do | body | + expect(JSON.parse(body)['error']).to include 'message' => "test error" + end + subject + end + + it "includes the backtrace in the error response" do + expect(response).to receive(:body=) do | body | + expect(body).to include("backtrace") + end + subject end - subject end end @@ -72,11 +99,47 @@ module Resources allow(PactBroker.configuration).to receive(:show_backtrace_in_error_response?).and_return(false) end - it "does not include the backtrace in the error response" do - expect(response).to receive(:body=) do | body | - expect(body).to_not include("backtrace") + context "when the error is a PactBroker::Error or subclass" do + let(:error) { Class.new(PactBroker::Error).new('test error') } + + it "does not invoke the api error reporters" do + expect(thing).to_not receive(:call).with(error, options) + subject + end + + it "uses the error message as the message" do + expect(response).to receive(:body=) do | body | + expect(JSON.parse(body)['error']).to include 'message' => "test error" + end + subject + end + + it "does not include the backtrace in the error response" do + expect(response).to receive(:body=) do | body | + expect(body).to_not include("backtrace") + end + subject + end + end + context "when the error is not a PactBroker::Error or subclass" do + it "invokes the api error reporters" do + expect(thing).to receive(:call).with(error, options) + subject + end + + it "uses a hardcoded error message" do + expect(response).to receive(:body=) do | body | + expect(JSON.parse(body)['error']['message']).to match /An error/ + end + subject + end + + it "does not include the backtrace in the error response" do + expect(response).to receive(:body=) do | body | + expect(body).to_not include("backtrace") + end + subject end - subject end end