From 57eed6503bfa16bb9b5012bf9032486e6a406081 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 6 Apr 2018 08:06:38 +1000
Subject: [PATCH] fix: do not invoke error reporters for validation errors

---
 .../api/resources/error_handler.rb            |  6 +++++-
 lib/pact_broker/api/resources/error_test.rb   |  4 ++--
 lib/pact_broker/error.rb                      |  1 +
 lib/pact_broker/ui/controllers/error_test.rb  |  2 +-
 .../api/resources/error_handler_spec.rb       | 20 ++++++++++++++++++-
 5 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/lib/pact_broker/api/resources/error_handler.rb b/lib/pact_broker/api/resources/error_handler.rb
index 6c8a972a3..82f58188d 100644
--- a/lib/pact_broker/api/resources/error_handler.rb
+++ b/lib/pact_broker/api/resources/error_handler.rb
@@ -12,7 +12,11 @@ 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
+          report(e, request) if reportable?(e)
+        end
+
+        def self.reportable? e
+          !e.is_a?(PactBroker::Error)
         end
 
         def self.report e, request
diff --git a/lib/pact_broker/api/resources/error_test.rb b/lib/pact_broker/api/resources/error_test.rb
index 07680db9e..e2a0e1903 100644
--- a/lib/pact_broker/api/resources/error_test.rb
+++ b/lib/pact_broker/api/resources/error_test.rb
@@ -24,11 +24,11 @@ def allowed_methods
         end
 
         def to_json
-          raise PactBroker::Error.new("Don't panic. This is a test API error.")
+          raise PactBroker::TestError.new("Don't panic. This is a test API error.")
         end
 
         def from_json
-          raise PactBroker::Error.new("Don't panic. This is a test API error.")
+          raise PactBroker::TestError.new("Don't panic. This is a test API error.")
         end
       end
     end
diff --git a/lib/pact_broker/error.rb b/lib/pact_broker/error.rb
index cf440b33a..6072d494c 100644
--- a/lib/pact_broker/error.rb
+++ b/lib/pact_broker/error.rb
@@ -1,5 +1,6 @@
 module PactBroker
 
   class Error < StandardError; end
+  class TestError < StandardError; end
 
 end
diff --git a/lib/pact_broker/ui/controllers/error_test.rb b/lib/pact_broker/ui/controllers/error_test.rb
index 2668d0b44..81624b328 100644
--- a/lib/pact_broker/ui/controllers/error_test.rb
+++ b/lib/pact_broker/ui/controllers/error_test.rb
@@ -9,7 +9,7 @@ class ErrorTest < Base
         include PactBroker::Services
 
         get "/" do
-          raise PactBroker::Error.new("Don't panic. This is a test UI error.")
+          raise PactBroker::TestError.new("Don't panic. This is a test UI error.")
         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
index c00c7562a..64656fa3f 100644
--- a/spec/lib/pact_broker/api/resources/error_handler_spec.rb
+++ b/spec/lib/pact_broker/api/resources/error_handler_spec.rb
@@ -5,7 +5,7 @@ module Api
     module Resources
       describe ErrorHandler do
         describe "call" do
-          let(:error) { PactBroker::Error.new('test error') }
+          let(:error) { StandardError.new('test error') }
           let(:thing) { double('thing', call: nil, another_call: nil) }
           let(:options) { { env: env } }
           let(:request) { double('request' ) }
@@ -31,6 +31,24 @@ 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
+
+          context "when the error is a PactBroker::TestError" do
+            let(:error) { PactBroker::TestError.new('test error') }
+
+            it "invokes the api error reporters" do
+              expect(thing).to receive(:call).with(error, options)
+              subject
+            end
+          end
+
           context "when the error reporter raises an error itself" do
             class TestError < StandardError; end