feat: Improve HTTP errors handling (pact-foundation#76)

Add more informative exception on HTTP 409 Conflict and improve generic
error exception message.

lib/pact_broker/client/base_client.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'erb'
 require 'httparty'
 require 'pact_broker/client/error'
@@ -30,6 +32,12 @@ def string_keys_to_symbols hash
     end
 
     class BaseClient
+      ERROR_CODE_MAPPING = {
+        401 => "Authentication failed",
+        403 => "Authorization failed (insufficient permissions)",
+        409 => "Potential duplicate pacticipants"
+      }.freeze
+
       include UrlHelpers
       include HTTParty
       include StringToSymbol
@@ -69,14 +77,8 @@ def handle_response response
           yield response
         elsif response.code == 404
           nil
-        elsif response.code == 403
-          message = "Authorization failed (insufficient permissions)"
-          if response.body && response.body.size > 0
-            message = message + ": #{response.body}"
-          end
-          raise Error.new(message)
-        elsif response.code == 401
-          message = "Authentication failed"
+        elsif ERROR_CODE_MAPPING.key?(response.code)
+          message = ERROR_CODE_MAPPING.fetch(response.code)
           if response.body && response.body.size > 0
             message = message + ": #{response.body}"
           end
@@ -93,7 +95,7 @@ def handle_response response
               response.body
             end
           rescue
-            raise Error.new(response.body)
+            raise Error.new("status=#{response.code} #{response.body}")
           end
           raise Error.new(error_message)
         end

lib/pact_broker/client/matrix.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require_relative 'base_client'
 require 'pact_broker/client/matrix/resource'
 
@@ -39,7 +41,7 @@ def handle_response response
               response.body
             end
           rescue
-            raise Error.new(response.body)
+            raise Error.new("status=#{response.code} #{response.body}")
           end
           raise Error.new(error_message)
         end

spec/lib/pact_broker/client/base_client_spec.rb

@@ -2,10 +2,11 @@
 module PactBroker
   module Client
     describe BaseClient do
-      describe '#initialize' do
-        subject { BaseClient.new(base_url: base_url) }
+      subject { BaseClient.new(base_url: base_url) }
+
+      let(:base_url) { 'http://pact_broker_base_url'}
 
-        let(:base_url) { 'http://pact_broker_base_url'}
+      describe '#initialize' do
         let(:username) { 'pact_repo_username'}
         let(:password) { 'pact_repo_password'}
         let(:token) { '123456789' }
@@ -119,6 +120,63 @@ module Client
           end
         end
       end
+
+      describe '#handle_response' do
+        let(:response) { double('Response', success?: true) }
+
+        it 'yields response object' do
+          expect { |block| subject.handle_response(response, &block) }.to yield_with_args(response)
+        end
+
+        context 'with 404 response' do
+          let(:response) { double('Response', success?: false, code: 404) }
+          it 'returns nil' do
+            expect(subject.handle_response(response)).to be_nil
+          end
+        end
+
+        context 'with 401 response' do
+          let(:response) { double('Response', success?: false, code: 401, body: 'body') }
+          it 'raise an exception with meaningful message' do
+            expect { subject.handle_response(response) }
+              .to raise_error(PactBroker::Client::Error, "Authentication failed: body")
+          end
+        end
+
+        context 'with 403 response' do
+          let(:response) { double('Response', success?: false, code: 403, body: 'body') }
+          it 'raise an exception with meaningful message' do
+            expect { subject.handle_response(response) }
+              .to raise_error(PactBroker::Client::Error, "Authorization failed (insufficient permissions): body")
+          end
+        end
+
+        context 'with 409 response' do
+          let(:response) { double('Response', success?: false, code: 409, body: 'body') }
+          it 'raise an exception with meaningful message' do
+            expect { subject.handle_response(response) }
+              .to raise_error(PactBroker::Client::Error, "Potential duplicate pacticipants: body")
+          end
+        end
+
+        context 'with unsuccessful JSON response' do
+          let(:response) do
+            double('Response', success?: false, code: 500, body: '{"errors": ["Internal server error"]}')
+          end
+          it 'raise an exception with meaningful message' do
+            expect { subject.handle_response(response) }
+              .to raise_error(PactBroker::Client::Error, "Internal server error")
+          end
+        end
+
+        context 'with unsucessful nono-JSON response ' do
+          let(:response) { double('Response', success?: false, code: 500, body: 'Internal server error') }
+          it 'raise an exception with meaningful message' do
+            expect { subject.handle_response(response) }
+              .to raise_error(PactBroker::Client::Error, "status=500 Internal server error")
+          end
+        end
+      end
     end
   end
-end
+end