From 6f7aef24030d9eb60ffb9ea474b3ade882f21add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Fri, 25 Jul 2025 11:11:53 +0200 Subject: [PATCH] feat: enhance CheckoutApiException to include request_id and improve error handling --- README.md | 4 +- checkout_sdk/exception.py | 20 +++-- tests/checkout_test_utils.py | 2 +- tests/exception_test.py | 78 ++++++++++++++++--- .../request_apm_payments_integration_test.py | 3 +- 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 3038e49..d278f49 100644 --- a/README.md +++ b/README.md @@ -176,13 +176,15 @@ def oauth(): ## Exception handling All the API responses that do not fall in the 2** status codes will cause a `CheckoutApiException`. The exception encapsulates -the `http_metadata` and a dictionary of `error_details`, if available. +the `http_metadata`, `request_id`, `error_type`, and a list of `error_details`, if available. ```python try: checkout_api.customers.get("customer_id") except CheckoutApiException as err: http_status_code = err.http_metadata.status_code + request_id = err.request_id + error_type = err.error_type error_details = err.error_details ``` diff --git a/checkout_sdk/exception.py b/checkout_sdk/exception.py index 4c0419b..0d7cc08 100644 --- a/checkout_sdk/exception.py +++ b/checkout_sdk/exception.py @@ -1,5 +1,7 @@ from __future__ import absolute_import +import logging + from checkout_sdk.authorization_type import AuthorizationType from checkout_sdk.utils import map_to_http_metadata @@ -33,21 +35,27 @@ def invalid_key(key_type: AuthorizationType): class CheckoutApiException(CheckoutException): http_metadata: dict + request_id: str error_details: list error_type: str def __init__(self, response): self.http_metadata = map_to_http_metadata(response) + self.request_id = None + self.error_details = None + self.error_type = None + if response.text: try: payload = response.json() + self.request_id = payload.get('request_id') self.error_details = payload.get('error_codes') self.error_type = payload.get('error_type') - except ValueError: - self.error_details = None - self.error_type = None - else: - self.error_details = None - self.error_type = None + except (ValueError, KeyError, TypeError) as e: + logging.error("Failed to parse response JSON payload: %s", e) + + if not self.request_id: + self.request_id = response.headers.get('Cko-Request-Id') + super().__init__('The API response status code ({}) does not indicate success.' .format(response.status_code)) diff --git a/tests/checkout_test_utils.py b/tests/checkout_test_utils.py index 2a67317..1dc6344 100644 --- a/tests/checkout_test_utils.py +++ b/tests/checkout_test_utils.py @@ -117,7 +117,7 @@ class VisaCard: name: str = 'Checkout Test' number: str = '4242424242424242' expiry_month: int = 6 - expiry_year: int = 2025 + expiry_year: int = 2030 cvv: str = '100' diff --git a/tests/exception_test.py b/tests/exception_test.py index 6ee5d7b..9f3aa2d 100644 --- a/tests/exception_test.py +++ b/tests/exception_test.py @@ -66,11 +66,13 @@ def test_invalid_authorization_various_types(auth_type_name): def test_checkout_api_exception(): response = Mock() response.status_code = 400 - response.text = '{"error_type": "request_invalid", "error_codes": ["invalid_field"]}' + response.text = '{"error_type": "request_invalid", "error_codes": ["invalid_field"], "request_id": "req_123456"}' response.json.return_value = { "error_type": "request_invalid", - "error_codes": ["invalid_field"] + "error_codes": ["invalid_field"], + "request_id": "req_123456" } + response.headers = {} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) @@ -78,15 +80,18 @@ def test_checkout_api_exception(): assert exception.http_metadata.status_code == 400 assert exception.error_type == "request_invalid" assert exception.error_details == ["invalid_field"] + assert exception.request_id == "req_123456" def test_checkout_api_exception_without_error_details(): response = Mock() response.status_code = 500 - response.text = '{"message": "Internal Server Error"}' + response.text = '{"message": "Internal Server Error", "request_id": "req_789012"}' response.json.return_value = { - "message": "Internal Server Error" + "message": "Internal Server Error", + "request_id": "req_789012" } + response.headers = {} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) @@ -94,13 +99,14 @@ def test_checkout_api_exception_without_error_details(): assert exception.http_metadata.status_code == 500 assert exception.error_type is None assert exception.error_details is None + assert exception.request_id == "req_789012" def test_checkout_api_exception_empty_response(): response = Mock() response.status_code = 404 response.text = '' - response.json.return_value = {} + response.headers = {'Cko-Request-Id': 'header_req_345678'} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) @@ -108,6 +114,7 @@ def test_checkout_api_exception_empty_response(): assert exception.http_metadata.status_code == 404 assert exception.error_type is None assert exception.error_details is None + assert exception.request_id == "header_req_345678" def test_checkout_api_exception_non_json_response(): @@ -115,6 +122,7 @@ def test_checkout_api_exception_non_json_response(): response.status_code = 502 response.text = 'Bad Gateway' response.json.side_effect = ValueError("No JSON object could be decoded") + response.headers = {'Cko-Request-Id': 'header_req_502502'} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) @@ -122,6 +130,39 @@ def test_checkout_api_exception_non_json_response(): assert exception.http_metadata.status_code == 502 assert exception.error_type is None assert exception.error_details is None + assert exception.request_id == "header_req_502502" + + +def test_checkout_api_exception_request_id_from_header_fallback(): + response = Mock() + response.status_code = 400 + response.text = '{"error_type": "request_invalid", "error_codes": ["invalid_field"]}' + response.json.return_value = { + "error_type": "request_invalid", + "error_codes": ["invalid_field"] + } + response.headers = {'Cko-Request-Id': '0120e756-6d00-453c-a398-ff1643f9a873'} + + with pytest.raises(CheckoutApiException) as exc_info: + raise CheckoutApiException(response) + exception = exc_info.value + assert exception.request_id == "0120e756-6d00-453c-a398-ff1643f9a873" + assert exception.error_type == "request_invalid" + assert exception.error_details == ["invalid_field"] + + +def test_checkout_api_exception_no_request_id_anywhere(): + response = Mock() + response.status_code = 400 + response.text = '{"error_type": "request_invalid"}' + response.json.return_value = {"error_type": "request_invalid"} + response.headers = {} # Sin Cko-Request-Id + + with pytest.raises(CheckoutApiException) as exc_info: + raise CheckoutApiException(response) + exception = exc_info.value + assert exception.request_id is None + assert exception.error_type == "request_invalid" @pytest.mark.parametrize("status_code", [400, 401, 403, 404, 500]) @@ -129,12 +170,13 @@ def test_checkout_api_exception_various_status_codes(status_code): response = Mock() response.status_code = status_code response.text = '' - response.json.return_value = {} + response.headers = {'Cko-Request-Id': f'req_{status_code}'} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) exception = exc_info.value assert exception.http_metadata.status_code == status_code + assert exception.request_id == f'req_{status_code}' def test_map_to_http_metadata(): @@ -150,24 +192,27 @@ def test_map_to_http_metadata(): def test_checkout_api_exception_message(): response = Mock() response.status_code = 400 - response.text = '{"error_type": "invalid_request", "error_codes": ["bad_request"]}' + response.text = '{"error_type": "invalid_request", "error_codes": ["bad_request"], "request_id": "msg_req_400"}' response.json.return_value = { "error_type": "invalid_request", - "error_codes": ["bad_request"] + "error_codes": ["bad_request"], + "request_id": "msg_req_400" } + response.headers = {} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) exception = exc_info.value expected_message = "The API response status code (400) does not indicate success." assert str(exception) == expected_message + assert exception.request_id == "msg_req_400" def test_checkout_api_exception_no_response_text(): response = Mock() response.status_code = 400 response.text = None - response.json.return_value = {} + response.headers = {'Cko-Request-Id': 'no_text_req_id'} with pytest.raises(CheckoutApiException) as exc_info: raise CheckoutApiException(response) @@ -175,3 +220,18 @@ def test_checkout_api_exception_no_response_text(): assert exception.http_metadata.status_code == 400 assert exception.error_type is None assert exception.error_details is None + assert exception.request_id == "no_text_req_id" + + +def test_checkout_api_exception_logs_on_json_parse_error(caplog): + response = Mock() + response.status_code = 502 + response.text = 'Bad Gateway' + response.json.side_effect = ValueError("No JSON object could be decoded") + response.headers = {'Cko-Request-Id': 'header_req_logging'} + + with caplog.at_level("ERROR"): + with pytest.raises(CheckoutApiException): + raise CheckoutApiException(response) + + assert any("Failed to parse response JSON payload" in m for m in caplog.messages) diff --git a/tests/payments/request_apm_payments_integration_test.py b/tests/payments/request_apm_payments_integration_test.py index 17937f2..e5bad76 100644 --- a/tests/payments/request_apm_payments_integration_test.py +++ b/tests/payments/request_apm_payments_integration_test.py @@ -223,7 +223,8 @@ def test_should_request_alipay_plus_payment(default_api): except CheckoutApiException as err: assert err.args[0] == 'The API response status code (422) does not indicate success.' assert err.error_type == 'invalid_request' - assert err.error_details[0] == 'reference_invalid' + assert err.error_details is not None and 'reference_invalid' in err.error_details + assert err.request_id is not None def test_should_make_przelewy24_payment(default_api):