From ef64c60d988fba577543677c2fd6119ed55ef069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:03:42 +0100 Subject: [PATCH] Enhance `parse_response` Method to Improve Error Handling and Logging --- lib/checkout_sdk/api_client.rb | 17 +++- spec/checkout_sdk/api_client_spec.rb | 91 +++++++++++++++++++ .../payments/contexts/contexts_helper.rb | 36 ++++++++ .../contexts/contexts_integration_spec.rb | 35 +------ 4 files changed, 144 insertions(+), 35 deletions(-) create mode 100644 spec/checkout_sdk/api_client_spec.rb diff --git a/lib/checkout_sdk/api_client.rb b/lib/checkout_sdk/api_client.rb index b168df3..176aa04 100644 --- a/lib/checkout_sdk/api_client.rb +++ b/lib/checkout_sdk/api_client.rb @@ -127,14 +127,25 @@ def upload(path, authorization, file_request) end def parse_response(response) - raise CheckoutApiException, response if response.status < 200 || response.status >= 400 + if response.status < 200 || response.status >= 400 + raise CheckoutApiException.new(response), + "The API response status code (#{response.status}) does not indicate success." + end 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 = OpenStruct.new(items: body) if body.is_a?(Array) + + 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 + raise CheckoutApiException.new(response), "Unexpected error: #{e.message}" end def parse_json_or_contents(response) diff --git a/spec/checkout_sdk/api_client_spec.rb b/spec/checkout_sdk/api_client_spec.rb new file mode 100644 index 0000000..4a158e2 --- /dev/null +++ b/spec/checkout_sdk/api_client_spec.rb @@ -0,0 +1,91 @@ +# 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"}') + allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).and_return({ metadata: 'test' }) + + parsed_response = api_client.send(:parse_response, response) + + expect(parsed_response.key).to eq('value') + expect(parsed_response.http_metadata[:metadata]).to eq('test') + end + + it 'returns an OpenStruct object with items for an array response' do + response = double('Response', status: 200, body: '[{"item":"value"}]') + allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).and_return({ metadata: 'test' }) + + 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[:metadata]).to eq('test') + end + end + + context 'when the response body is nil' do + it 'returns an empty OpenStruct' do + response = double('Response', status: 200, body: nil) + allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).and_return({ metadata: 'test' }) + + parsed_response = api_client.send(:parse_response, response) + + expect(parsed_response).to be_a(OpenStruct) + expect(parsed_response.http_metadata[:metadata]).to eq('test') + end + end + + context 'when the response status is less than 200' do + it 'raises a CheckoutApiException' do + response = double('Response', status: 199, body: '{}') + + expect do + api_client.send(:parse_response, response) + end.to raise_error(CheckoutSdk::CheckoutApiException, /Invalid response status: 199/) + end + end + + context 'when the response status is 400 or higher' do + it 'raises a CheckoutApiException for client errors (4xx)' do + response = double('Response', status: 400, body: '{}') + + expect do + api_client.send(:parse_response, response) + end.to raise_error(CheckoutSdk::CheckoutApiException, /Invalid response status: 400/) + end + + it 'raises a CheckoutApiException for server errors (5xx)' do + response = double('Response', status: 500, body: '{}') + + expect do + api_client.send(:parse_response, response) + end.to raise_error(CheckoutSdk::CheckoutApiException, /Invalid response status: 500/) + 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}') + + allow(CheckoutSdk::CheckoutUtils).to receive(:map_to_http_metadata).with(response).and_return(OpenStruct.new(metadata: 'test')) + + expect do + api_client.send(:parse_response, response) + end.to raise_error(CheckoutSdk::CheckoutApiException, /Error parsing JSON/) + end + end + end +end diff --git a/spec/checkout_sdk/payments/contexts/contexts_helper.rb b/spec/checkout_sdk/payments/contexts/contexts_helper.rb index 42cba44..df944ca 100644 --- a/spec/checkout_sdk/payments/contexts/contexts_helper.rb +++ b/spec/checkout_sdk/payments/contexts/contexts_helper.rb @@ -31,4 +31,40 @@ def create_payment_contexts_paypal response end + def create_payment_contexts_klarna + request = { + 'source' => { + 'type' => 'klarna', + 'account_holder' => { + 'billing_address' => { + 'country' => 'DE' + } + } + }, + 'amount' => 1000, + 'currency' => CheckoutSdk::Common::Currency::EUR, + 'payment_type' => CheckoutSdk::Payments::PaymentType::REGULAR, + 'processing_channel_id' => ENV.fetch('CHECKOUT_PROCESSING_CHANNEL_ID', nil), + 'items' => [ + { + 'name' => 'mask', + 'unit_price' => 1000, + 'quantity' => 1, + 'total_amount' => 1000, + 'reference' => 'BA67A' + } + ], + 'processing' => { + 'locale' => 'en-GB' + } + } + + response = default_sdk.contexts.create_payment_contexts(request) + expect(response).not_to be nil + expect(response.id).not_to be nil + expect(response.partner_metadata.client_token).not_to be nil + expect(response.partner_metadata.session_id).not_to be nil + response + end + end diff --git a/spec/checkout_sdk/payments/contexts/contexts_integration_spec.rb b/spec/checkout_sdk/payments/contexts/contexts_integration_spec.rb index d93def6..64822cb 100644 --- a/spec/checkout_sdk/payments/contexts/contexts_integration_spec.rb +++ b/spec/checkout_sdk/payments/contexts/contexts_integration_spec.rb @@ -5,6 +5,7 @@ before(:all) do @payment_context_paypal = create_payment_contexts_paypal + @payment_context_klarna = create_payment_contexts_klarna end describe '.create_payment_contexts' do @@ -12,36 +13,7 @@ it { is_valid_payment_context @payment_context_paypal } end context 'when creating a payment contexts klarna with valid data' do - it 'raises an error (apm_service_unavailable)' do - request = { - 'source' => { - 'type' => 'klarna', - 'account_holder' => { - 'billing_address' => { - 'country' => 'DE' - } - } - }, - 'amount' => 1000, - 'currency' => CheckoutSdk::Common::Currency::EUR, - 'payment_type' => CheckoutSdk::Payments::PaymentType::REGULAR, - 'processing_channel_id' => ENV.fetch('CHECKOUT_PROCESSING_CHANNEL_ID', nil), - 'items' => [ - { - 'name' => 'mask', - 'unit_price' => 1000, - 'quantity' => 1, - 'total_amount' => 1000, - 'reference' => 'BA67A' - } - ], - 'processing' => { - 'locale' => 'en-GB' - } - } - expect { default_sdk.contexts.create_payment_contexts(request) } - .to raise_error(CheckoutSdk::CheckoutApiException) { |e| expect(e.error_details[:error_codes].first).to eq 'apm_service_unavailable' } - end + it { is_valid_payment_context @payment_context_klarna } end end @@ -54,8 +26,7 @@ def is_valid_payment_context(payment_context) assert_response payment_context, %w[id - partner_metadata - partner_metadata.order_id] + partner_metadata] end def is_valid_payment_context_details(payment_context_details_response)