Change to expose data and error_messages in reference to issue piotrmurach#273

piotrmurach · piotrmurach · commit dceb5e14d31a · 2016-07-15T23:24:15.000+02:00
diff --git a/lib/github_api/error/service_error.rb b/lib/github_api/error/service_error.rb
@@ -31,8 +31,6 @@ def self.error_mapping
 
       MIN_BODY_LENGTH = 2
 
-      attr_reader :errors
-
       # Crate a ServiceError
       #
       # @param [Hash[Symbol]] response
@@ -49,6 +47,27 @@ def initialize(response)
         super(create_message(response))
       end
 
+      # Expose response payload as JSON object if possible
+      #
+      # @return [Hash[Symbol]|String]
+      #
+      # @api public
+      def data
+        @data ||= decode_data(@body)
+      end
+
+      # Stores error message(s) returned in response body
+      #
+      # @return [Array[Hash[Symbol]]]
+      #   the array of hash error objects
+      #
+      # @api public
+      def error_messages
+        @error_messages ||= begin
+          data[:errors] ? data[:errors] : [data]
+        end
+      end
+
       private
 
       # Create full error message
@@ -65,7 +84,7 @@ def create_message(response)
 
         message = "#{response[:method].to_s.upcase} "
         message << "#{response[:url]}: "
-        message << "#{@status} - #{parse_body(@body)}"
+        message << "#{@status} - #{format_response}"
         message
       end
 
@@ -86,22 +105,23 @@ def decode_data(body)
         end
       end
 
+      # Read response body and convert to human friendly format
+      #
+      # @return [String]
+      #
       # @api private
-      def parse_body(body)
-        data = decode_data(body)
-
+      def format_response
         return '' if data.nil? || data.empty?
 
         case data
         when Hash
           message = data[:message] ? data[:message] : ' '
           docs = data[:documentation_url]
-          error = create_error_summary(data)
+          error = create_error_summary
           message << error if error
           message << "\nSee: #{docs}" if docs
           message
         when String
-          @errors = [data]
           data
         end
       end
@@ -111,19 +131,14 @@ def parse_body(body)
       # @return [String]
       #
       # @api private
-      def create_error_summary(data)
+      def create_error_summary
         if data[:error]
-          @errors = [data]
-          return "\nError: #{data[:error]}"
+          "\nError: #{data[:error]}"
         elsif data[:errors]
-          @errors = data[:errors]
           message = "\nErrors:\n"
           message << data[:errors].map do |error|
             "Error: #{error.map { |k, v| "#{k}: #{v}" }.join(', ')}"
           end.join("\n")
-        else
-          @errors = [data]
-          nil
         end
       end
     end # ServiceError
diff --git a/spec/unit/error/service_error/new_spec.rb b/spec/unit/error/service_error/new_spec.rb
@@ -27,7 +27,7 @@ def create_response(body)
     expect(error.to_s).to eq("POST #{url}: #{status} - Requires authentication")
     expect(error.response_headers).to eql(response_headers)
     expect(error.response_message).to eql(body)
-    expect(error.errors).to eql([{ message: 'Requires authentication' }])
+    expect(error.error_messages).to eql([{ message: 'Requires authentication' }])
   end
 
   it "creates message with single error summary" do
@@ -43,7 +43,7 @@ def create_response(body)
     ].join("\n"))
     expect(error.response_headers).to eql(response_headers)
     expect(error.response_message).to eql(body)
-    expect(error.errors).to eql([{:message=>"Validation Failed\nError: No commits between master and noexist\nSee: https://developer.github.com/enterprise/2.6/v3/pulls/#create-a-pull-request", :error=>"No commits between master and noexist", :documentation_url=>"https://developer.github.com/enterprise/2.6/v3/pulls/#create-a-pull-request"}])
+    expect(error.error_messages).to eql([{:message=>"Validation Failed\nError: No commits between master and noexist\nSee: https://developer.github.com/enterprise/2.6/v3/pulls/#create-a-pull-request", :error=>"No commits between master and noexist", :documentation_url=>"https://developer.github.com/enterprise/2.6/v3/pulls/#create-a-pull-request"}])
   end
 
   it "creates message with multiple errors summary" do
@@ -62,6 +62,9 @@ def create_response(body)
     ].join("\n"))
     expect(error.response_headers).to eql(response_headers)
     expect(error.response_message).to eql(body)
-    expect(error.errors).to eql([{:resource=>"PullRequest", :code=>"missing_field", :field=>"head_sha"}, {:resource=>"PullRequest", :code=>"missing_field", :field=>"base_sha"}, {:resource=>"PullRequest", :code=>"custom", :message=>"No commits between master and noexist"}])
+    expect(error.error_messages).to eql([
+      {:resource=>"PullRequest", :code=>"missing_field", :field=>"head_sha"},
+      {:resource=>"PullRequest", :code=>"missing_field", :field=>"base_sha"},
+      {:resource=>"PullRequest", :code=>"custom", :message=>"No commits between master and noexist"}])
   end
 end # Github::Error::ServiceError
