Enhance parse_response Method to Improve Error Handling and Logging

lib/checkout_sdk/api_client.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
+require 'csv'
+
 module CheckoutSdk
   class ApiClient
     attr_accessor :client, :multipart_client, :log
 
-    # @param [CheckoutConfiguration] configuration
-    # @param [String] uri
     def initialize(configuration, uri)
       @client = configuration.http_client.clone
       @client.url_prefix = uri
@@ -14,42 +14,27 @@ def initialize(configuration, uri)
       @log = configuration.logger
     end
 
-    # @param [String] path
-    # @param [SdkAuthorization] authorization
-    # @param [Object] params
-    def invoke_get(path,
-                   authorization,
-                   params = nil)
+    def invoke_get(path, authorization, params = nil)
       invoke(:get, path, authorization, params: params)
     end
 
-    def invoke_post(path,
-                    authorization,
-                    request = nil,
-                    idempotency_key = nil)
+    def invoke_post(path, authorization, request = nil, idempotency_key = nil)
       invoke(:post, path, authorization, request, idempotency_key)
     end
 
-    def invoke_put(path,
-                   authorization,
-                   request)
+    def invoke_put(path, authorization, request)
       invoke(:put, path, authorization, request)
     end
 
-    def invoke_patch(path,
-                     authorization,
-                     request = nil)
+    def invoke_patch(path, authorization, request = nil)
       invoke(:patch, path, authorization, request)
     end
 
-    def invoke_delete(path,
-                      authorization)
+    def invoke_delete(path, authorization)
       invoke(:delete, path, authorization)
     end
 
-    def submit_file(path,
-                    authorization,
-                    request)
+    def submit_file(path, authorization, request)
       upload(path, authorization, request)
     end
 
@@ -58,7 +43,7 @@ def submit_file(path,
     def invoke(method, path, authorization, body = nil, idempotency_key = nil, params: nil)
       path = append_params(path, params) unless params.nil?
 
-      headers = get_default_headers authorization
+      headers = default_headers(authorization)
       headers[:'Content-Type'] = 'application/json'
       headers[:'Cko-Idempotency-Key'] = idempotency_key unless idempotency_key.nil?
 
@@ -71,26 +56,23 @@ def invoke(method, path, authorization, body = nil, idempotency_key = nil, param
         raise CheckoutApiException, e.response
       end
 
-      parse_response response
+      parse_response(response)
     end
 
-    def get_default_headers(authorization)
-      {
-        'User-Agent': "checkout-sdk-ruby/#{VERSION}",
-        Accept: 'application/json',
-        Authorization: authorization.authorization_header
-      }
+    def default_headers(authorization)
+      { 'User-Agent': "checkout-sdk-ruby/#{VERSION}", Accept: 'application/json',
+        Authorization: authorization.authorization_header }
     end
 
     def append_params(path, input_params)
       raise CheckoutArgumentException, 'Query parameters were not provided' if input_params.nil?
 
-      if input_params.is_a? String
-        params = input_params
-      else
-        hash = CheckoutSdk::JsonSerializer.to_custom_hash(input_params)
-        params = URI.encode_www_form(hash)
-      end
+      params = if input_params.is_a? String
+                 input_params
+               else
+                 hash = CheckoutSdk::JsonSerializer.to_custom_hash(input_params)
+                 URI.encode_www_form(hash)
+               end
 
       "#{path}?#{params}"
     end
@@ -103,16 +85,16 @@ def build_multipart_request(file_request, file)
           MIME::Types.type_for(file_request.file).first,
           File.basename(file_request.file)
         ),
-        :purpose => file_request.purpose
+        purpose: file_request.purpose
       }
     end
 
     def upload(path, authorization, file_request)
-      headers = get_default_headers authorization
+      headers = default_headers(authorization)
 
       file = File.open(file_request.file)
 
-      form = build_multipart_request file_request, file
+      form = build_multipart_request(file_request, file)
 
       begin
         @log.info "post: /#{path}"
@@ -123,28 +105,55 @@ def upload(path, authorization, file_request)
         file.close
       end
 
-      parse_response response
+      parse_response(response)
     end
 
     def parse_response(response)
-      raise CheckoutApiException, response if response.status < 200 || response.status >= 400
+      raise CheckoutApiException, response if response.status < 200 || response.status >= 300
 
       metadata = CheckoutUtils.map_to_http_metadata(response)
-      body = parse_json_or_contents(response)
-      body = OpenStruct.new if body.nil?
-      body = OpenStruct.new(items: body) if body.is_a? Array
-      body.http_metadata = metadata if body.is_a? OpenStruct
+      body = parse_body(response)
+
+      if body.is_a?(Array)
+        body = OpenStruct.new(items: body)
+      elsif !body.is_a?(OpenStruct)
+        body = OpenStruct.new(contents: body)
+      end
+
+      body.http_metadata = metadata if body.is_a?(OpenStruct)
+
       body
+    rescue JSON::ParserError => e
+      raise CheckoutApiException.new(response, "Error parsing JSON: #{e.message}")
+    rescue StandardError => e
+      @log&.error("Unexpected error occurred: #{e.message}")
+      raise
     end
 
-    def parse_json_or_contents(response)
-      return if response.body.nil? || response.body == ''
+    def parse_body(response)
+      content_type = response.headers['Content-Type']
+      return OpenStruct.new if response.body.nil? || response.body.empty?
 
-      if response.body.start_with?('{', '[')
-        JSON.parse(response.body, object_class: OpenStruct)
+      if content_type&.include?('application/json')
+        parsed_value = JSON.parse(response.body)
+        deep_convert_to_ostruct(parsed_value)
+      elsif content_type&.include?('text/csv')
+        csv_data = CSV.parse(response.body, headers: true)
+        OpenStruct.new(csv: csv_data)
       else
         OpenStruct.new(contents: response.body)
       end
     end
+
+    def deep_convert_to_ostruct(obj)
+      case obj
+      when Hash
+        OpenStruct.new(obj.transform_values { |value| deep_convert_to_ostruct(value) })
+      when Array
+        obj.map { |item| deep_convert_to_ostruct(item) }
+      else
+        obj
+      end
+    end
   end
 end

lib/checkout_sdk/error.rb

@@ -1,30 +1,46 @@
 # frozen_string_literal: true
 
 module CheckoutSdk
-  class CheckoutException < StandardError
-  end
+  class CheckoutException < StandardError; end
 
   class CheckoutArgumentException < CheckoutException; end
 
   class CheckoutAuthorizationException < CheckoutException
     def self.invalid_authorization(authorization_type)
-      CheckoutAuthorizationException.new("Operation requires #{authorization_type} authorization type")
+      new("Operation requires #{authorization_type} authorization type.")
     end
 
     def self.invalid_key(key_type)
-      CheckoutAuthorizationException.new("#{key_type} is required for this operation.")
+      new("#{key_type} is required for this operation.")
     end
   end
 
   class CheckoutApiException < CheckoutException
     attr_reader :http_metadata, :error_details
 
-    def initialize(response)
+    def initialize(response, message = nil)
       @http_metadata = CheckoutUtils.map_to_http_metadata(response)
-      if !http_metadata.body.nil? && http_metadata.body != ''
-        @error_details = JSON.parse(http_metadata.body, object_class: OpenStruct)
+      @error_details = parse_error_details(http_metadata.body)
+      super(message || build_error_message)
+    end
+
+    private
+
+    def parse_error_details(body)
+      return if body.nil? || body.empty?
+
+      JSON.parse(body, object_class: OpenStruct)
+    rescue JSON::ParserError
+      nil
+    end
+
+    def build_error_message
+      message = "The API response status code (#{http_metadata.status_code}) does not indicate success."
+      if @error_details && !@error_details.to_h.empty?
+        details = @error_details.to_h.map { |key, value| "#{key}: #{value}" }.join(', ')
+        message += " Details: #{details}."
       end
-      super("The API response status code (#{http_metadata.status_code}) does not indicate success.")
+      message
     end
   end
 end

spec/checkout_sdk/api_client_spec.rb

@@ -0,0 +1,141 @@
+# frozen_string_literal: true
+
+RSpec.describe CheckoutSdk::ApiClient do
+  let(:configuration) do
+    double(
+      'CheckoutConfiguration',
+      http_client: Faraday.new,
+      multipart_http_client: Faraday.new,
+      logger: Logger.new(STDOUT)
+    )
+  end
+
+  let(:api_client) { CheckoutSdk::ApiClient.new(configuration, 'https://api.sandbox.checkout.com') }
+
+  describe '#parse_response' do
+    context 'when the response is successful' do
+      it 'parses the response correctly for valid JSON' do
+        response = double('Response', status: 200, body: '{"key":"value"}', headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200, body: response.body)
+        )
+        parsed_response = api_client.send(:parse_response, response)
+        expect(parsed_response.key).to eq('value')
+        expect(parsed_response.http_metadata.status_code).to eq(200)
+      end
+
+      it 'returns an OpenStruct object with items for an array response' do
+        response = double('Response', status: 200, body: '[{"item":"value"}]', headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200, body: response.body)
+        )
+        parsed_response = api_client.send(:parse_response, response)
+        expect(parsed_response.items).to be_an(Array)
+        expect(parsed_response.items.first.item).to eq('value')
+        expect(parsed_response.http_metadata.status_code).to eq(200)
+      end
+
+      it 'wraps primitive JSON values in OpenStruct with contents key' do
+        response = double('Response', status: 200, body: '123', headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200, body: response.body)
+        )
+        parsed_response = api_client.send(:parse_response, response)
+        expect(parsed_response.contents).to eq(123)
+        expect(parsed_response.http_metadata.status_code).to eq(200)
+      end
+    end
+
+    context 'when the response body is nil' do
+      it 'returns an empty OpenStruct' do
+        response = double('Response', status: 200, body: nil, headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200)
+        )
+        parsed_response = api_client.send(:parse_response, response)
+        expect(parsed_response).to be_a(OpenStruct)
+        expect(parsed_response.to_h.keys).to contain_exactly(:http_metadata)
+        expect(parsed_response.http_metadata.status_code).to eq(200)
+      end
+    end
+
+    context 'when the response body is an empty string' do
+      it 'returns an empty OpenStruct' do
+        response = double('Response', status: 200, body: '', headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200)
+        )
+        parsed_response = api_client.send(:parse_response, response)
+        expect(parsed_response).to be_a(OpenStruct)
+        expect(parsed_response.to_h.keys).to contain_exactly(:http_metadata)
+        expect(parsed_response.http_metadata.status_code).to eq(200)
+      end
+    end
+
+    context 'when the response status is not in the 2xx range' do
+      it 'raises a CheckoutApiException for status code less than 200' do
+        response = double('Response', status: 199, body: '{}')
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 199, body: response.body)
+        )
+        expect do
+          api_client.send(:parse_response, response)
+        end.to raise_error(CheckoutSdk::CheckoutApiException) do |error|
+          expect(error.message).to eq("The API response status code (199) does not indicate success.")
+          expect(error.http_metadata.status_code).to eq(199)
+        end
+      end
+
+      it 'raises a CheckoutApiException for client errors (4xx)' do
+        response = double('Response', status: 400, body: '{"error":"Bad Request"}')
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 400, body: response.body)
+        )
+        expect do
+          api_client.send(:parse_response, response)
+        end.to raise_error(CheckoutSdk::CheckoutApiException) do |error|
+          expect(error.message).to include("The API response status code (400) does not indicate success.")
+          expect(error.error_details.error).to eq('Bad Request')
+          expect(error.http_metadata.status_code).to eq(400)
+        end
+      end
+
+      it 'raises a CheckoutApiException for server errors (5xx)' do
+        response = double('Response', status: 500, body: '{"error":"Server Error"}')
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 500, body: response.body)
+        )
+        expect do
+          api_client.send(:parse_response, response)
+        end.to raise_error(CheckoutSdk::CheckoutApiException) do |error|
+          expect(error.message).to include("The API response status code (500) does not indicate success.")
+          expect(error.error_details.error).to eq('Server Error')
+          expect(error.http_metadata.status_code).to eq(500)
+        end
+      end
+    end
+
+    context 'when the response body is invalid JSON' do
+      it 'raises a CheckoutApiException with JSON parsing error' do
+        response = double('Response', status: 200, body: '{invalid_json}', headers: { 'Content-Type' => 'application/json' })
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(
+          OpenStruct.new(status_code: 200, body: response.body)
+        )
+        expect do
+          api_client.send(:parse_response, response)
+        end.to raise_error(CheckoutSdk::CheckoutApiException, /Error parsing JSON: .*/)
+      end
+    end
+
+    context 'when an unexpected exception occurs' do
+      it 'logs the error and re-raises the exception' do
+        response = double('Response', status: 200, body: '{"key":"value"}')
+        allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).and_raise(StandardError.new("Unexpected failure"))
+        expect(api_client.log).to receive(:error).with("Unexpected error occurred: Unexpected failure")
+        expect do
+          api_client.send(:parse_response, response)
+        end.to raise_error(StandardError, "Unexpected failure")
+      end
+    end
+  end
+end