From 5893273bd184dfbc8619040e01e6b52871502e67 Mon Sep 17 00:00:00 2001 From: moeez96 Date: Fri, 12 May 2023 14:36:28 +0500 Subject: [PATCH 1/2] refactor: Improve exception handling for mobile IAP --- ecommerce/extensions/iap/api/v1/constants.py | 3 +- .../extensions/iap/api/v1/tests/test_views.py | 53 ++++++++++--------- ecommerce/extensions/iap/api/v1/views.py | 37 +++++++------ 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/ecommerce/extensions/iap/api/v1/constants.py b/ecommerce/extensions/iap/api/v1/constants.py index b43407b2b61..e4ecf4c292f 100644 --- a/ecommerce/extensions/iap/api/v1/constants.py +++ b/ecommerce/extensions/iap/api/v1/constants.py @@ -20,8 +20,7 @@ LOGGER_BASKET_NOT_FOUND = "Basket [%s] not found for user [%s]." LOGGER_EXECUTE_ALREADY_PURCHASED = "Execute payment failed for user [%s] and basket [%s]. " \ "Products already purchased." -LOGGER_EXECUTE_ERROR_WHILE_OBTAINING_BASKET = "Execute payment failed for user [%s] and basket [%s]. " \ - "Fetching basket failed with error [%s]." +LOGGER_EXECUTE_GATEWAY_ERROR = "Execute payment validation failed for user [%s] and basket [%s]. Error: [%s]" LOGGER_EXECUTE_ORDER_CREATION_FAILED = "Execute payment failed for user [%s] and basket [%s]. " \ "Order Creation failed with error [%s]." LOGGER_EXECUTE_PAYMENT_ERROR = "Execute payment failed for user [%s] and basket [%s]. " \ diff --git a/ecommerce/extensions/iap/api/v1/tests/test_views.py b/ecommerce/extensions/iap/api/v1/tests/test_views.py index e1d0ca12f0f..748593e09d1 100644 --- a/ecommerce/extensions/iap/api/v1/tests/test_views.py +++ b/ecommerce/extensions/iap/api/v1/tests/test_views.py @@ -11,7 +11,7 @@ from django.urls import reverse from oauth2client.service_account import ServiceAccountCredentials from oscar.apps.order.exceptions import UnableToPlaceOrder -from oscar.apps.payment.exceptions import PaymentError +from oscar.apps.payment.exceptions import GatewayError, PaymentError from oscar.core.loading import get_class, get_model from oscar.test.factories import BasketFactory from rest_framework import status @@ -42,7 +42,7 @@ LOGGER_BASKET_CREATION_FAILED, LOGGER_BASKET_NOT_FOUND, LOGGER_EXECUTE_ALREADY_PURCHASED, - LOGGER_EXECUTE_ERROR_WHILE_OBTAINING_BASKET, + LOGGER_EXECUTE_GATEWAY_ERROR, LOGGER_EXECUTE_ORDER_CREATION_FAILED, LOGGER_EXECUTE_PAYMENT_ERROR, LOGGER_EXECUTE_REDUNDANT_PAYMENT, @@ -322,6 +322,30 @@ def test_payment_error(self): ), ) + def test_gateway_error(self): + """ + Verify that an error is thrown when an approved payment fails to execute + """ + with mock.patch.object(MobileCoursePurchaseExecutionView, 'handle_payment', + side_effect=GatewayError('Test Error')) as fake_handle_payment: + with LogCapture(self.logger_name) as logger: + self._assert_response({'error': ERROR_DURING_PAYMENT_HANDLING}) + self.assertTrue(fake_handle_payment.called) + + logger.check( + ( + self.logger_name, + 'INFO', + LOGGER_EXECUTE_STARTED % (self.user.username, self.basket.id, self.processor_name) + ), + ( + self.logger_name, + 'ERROR', + LOGGER_EXECUTE_GATEWAY_ERROR % (self.user.username, self.basket.id, + str(fake_handle_payment.side_effect)) + ), + ) + def test_unanticipated_error_during_payment_handling(self): """ Verify that a user who has approved payment is redirected to the configured receipt @@ -435,29 +459,6 @@ def test_payment_error_with_no_basket(self): ) ) - def test_payment_error_with_unanticipated_error_while_getting_basket(self): - """ - Verify that we fail gracefully when an unanticipated Exception occurred while - getting the basket. - """ - with mock.patch.object(MobileCoursePurchaseExecutionView, '_get_basket', side_effect=KeyError('Test error')) \ - as fake_get_basket, \ - LogCapture(self.logger_name) as logger: - self._assert_response({'error': ERROR_WHILE_OBTAINING_BASKET_FOR_USER.format(self.user.email)}) - logger.check_present( - ( - self.logger_name, - 'INFO', - LOGGER_EXECUTE_STARTED % (self.user.username, self.basket.id, self.processor_name) - ), - ( - self.logger_name, - 'ERROR', - LOGGER_EXECUTE_ERROR_WHILE_OBTAINING_BASKET % (self.user.username, self.basket.id, - str(fake_get_basket.side_effect)) - ), - ) - def test_iap_payment_execution_ios(self): """ Verify that a user gets successful response if payment is handled correctly and @@ -574,7 +575,7 @@ def test_redundant_payment_notification_error(self, mock_handle_payment): @mock.patch('ecommerce.extensions.checkout.mixins.EdxOrderPlacementMixin.handle_post_order') def test_post_order_exception(self, mock_handle_post_order): - mock_handle_post_order.side_effect = ValueError() + mock_handle_post_order.side_effect = AttributeError() expected_response_status_code = 200 error_message = ERROR_DURING_POST_ORDER_OP.encode('UTF-8') expected_response_content = b'{"error": "%s"}' % error_message diff --git a/ecommerce/extensions/iap/api/v1/views.py b/ecommerce/extensions/iap/api/v1/views.py index 414b8739112..a45feeb06f2 100644 --- a/ecommerce/extensions/iap/api/v1/views.py +++ b/ecommerce/extensions/iap/api/v1/views.py @@ -14,7 +14,7 @@ from googleapiclient.discovery import build from oauth2client.service_account import ServiceAccountCredentials from oscar.apps.basket.views import * # pylint: disable=wildcard-import, unused-wildcard-import -from oscar.apps.payment.exceptions import PaymentError +from oscar.apps.payment.exceptions import GatewayError, PaymentError from oscar.core.loading import get_class, get_model from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -49,7 +49,7 @@ LOGGER_BASKET_CREATION_FAILED, LOGGER_BASKET_NOT_FOUND, LOGGER_EXECUTE_ALREADY_PURCHASED, - LOGGER_EXECUTE_ERROR_WHILE_OBTAINING_BASKET, + LOGGER_EXECUTE_GATEWAY_ERROR, LOGGER_EXECUTE_ORDER_CREATION_FAILED, LOGGER_EXECUTE_PAYMENT_ERROR, LOGGER_EXECUTE_REDUNDANT_PAYMENT, @@ -224,23 +224,22 @@ def post(self, request): except AlreadyPlacedOrderException: logger.exception(LOGGER_EXECUTE_ALREADY_PURCHASED, request.user.username, basket_id) return JsonResponse({'error': _(ERROR_ALREADY_PURCHASED)}, status=406) - except Exception as exc: # pylint: disable=broad-except - logger.exception(LOGGER_EXECUTE_ERROR_WHILE_OBTAINING_BASKET, request.user.username, basket_id, str(exc)) - return JsonResponse({'error': ERROR_WHILE_OBTAINING_BASKET_FOR_USER.format(request.user.email)}, status=400) - try: - with transaction.atomic(): - try: - self.handle_payment(receipt, basket) - except RedundantPaymentNotificationError: - logger.exception(LOGGER_EXECUTE_REDUNDANT_PAYMENT, request.user.username, basket_id) - return JsonResponse({'error': COURSE_ALREADY_PAID_ON_DEVICE}, status=409) - except PaymentError as exception: - logger.exception(LOGGER_EXECUTE_PAYMENT_ERROR, request.user.username, basket_id, str(exception)) - return JsonResponse({'error': ERROR_DURING_PAYMENT_HANDLING}, status=400) - except Exception as exception: # pylint: disable=broad-except - logger.exception(LOGGER_PAYMENT_FAILED_FOR_BASKET, basket_id, str(exception)) - return JsonResponse({'error': ERROR_DURING_PAYMENT_HANDLING}, status=400) + with transaction.atomic(): + try: + self.handle_payment(receipt, basket) + except GatewayError as exception: + logger.exception(LOGGER_EXECUTE_GATEWAY_ERROR, request.user.username, basket_id, str(exception)) + return JsonResponse({'error': ERROR_DURING_PAYMENT_HANDLING}, status=400) + except KeyError as exception: + logger.exception(LOGGER_PAYMENT_FAILED_FOR_BASKET, basket_id, str(exception)) + return JsonResponse({'error': ERROR_DURING_PAYMENT_HANDLING}, status=400) + except RedundantPaymentNotificationError: + logger.exception(LOGGER_EXECUTE_REDUNDANT_PAYMENT, request.user.username, basket_id) + return JsonResponse({'error': COURSE_ALREADY_PAID_ON_DEVICE}, status=409) + except PaymentError as exception: + logger.exception(LOGGER_EXECUTE_PAYMENT_ERROR, request.user.username, basket_id, str(exception)) + return JsonResponse({'error': ERROR_DURING_PAYMENT_HANDLING}, status=400) try: order = self.create_order(request, basket) @@ -250,7 +249,7 @@ def post(self, request): try: self.handle_post_order(order) - except Exception: # pylint: disable=broad-except + except AttributeError: self.log_order_placement_exception(basket.order_number, basket.id) return JsonResponse({'error': ERROR_DURING_POST_ORDER_OP}, status=200) From d4b78f0d7cc3b4b7007da23404b4fc7be7f155ee Mon Sep 17 00:00:00 2001 From: moeez96 Date: Fri, 12 May 2023 14:39:43 +0500 Subject: [PATCH 2/2] refactor: pylint fixes --- ecommerce/extensions/iap/api/v1/constants.py | 1 - ecommerce/extensions/iap/api/v1/tests/test_views.py | 1 - ecommerce/extensions/iap/api/v1/views.py | 1 - 3 files changed, 3 deletions(-) diff --git a/ecommerce/extensions/iap/api/v1/constants.py b/ecommerce/extensions/iap/api/v1/constants.py index e4ecf4c292f..5aa6bb032f8 100644 --- a/ecommerce/extensions/iap/api/v1/constants.py +++ b/ecommerce/extensions/iap/api/v1/constants.py @@ -12,7 +12,6 @@ ERROR_REFUND_NOT_COMPLETED = "Could not complete refund for user [%s] in course [%s] by processor [%s]" ERROR_TRANSACTION_NOT_FOUND_FOR_REFUND = "Could not find any transaction to refund for [%s] by processor [%s]" ERROR_DURING_POST_ORDER_OP = "An error occurred during post order operations." -ERROR_WHILE_OBTAINING_BASKET_FOR_USER = "An unexpected exception occurred while obtaining basket for user [{}]." GOOGLE_PUBLISHER_API_SCOPE = "https://www.googleapis.com/auth/androidpublisher" LOGGER_BASKET_ALREADY_PURCHASED = "Basket creation failed for user [%s] with SKUS [%s]. Products already purchased" LOGGER_BASKET_CREATED = "Basket created for user [%s] with SKUS [%s]" diff --git a/ecommerce/extensions/iap/api/v1/tests/test_views.py b/ecommerce/extensions/iap/api/v1/tests/test_views.py index 748593e09d1..0da216b32f4 100644 --- a/ecommerce/extensions/iap/api/v1/tests/test_views.py +++ b/ecommerce/extensions/iap/api/v1/tests/test_views.py @@ -36,7 +36,6 @@ ERROR_ORDER_NOT_FOUND_FOR_REFUND, ERROR_REFUND_NOT_COMPLETED, ERROR_TRANSACTION_NOT_FOUND_FOR_REFUND, - ERROR_WHILE_OBTAINING_BASKET_FOR_USER, LOGGER_BASKET_ALREADY_PURCHASED, LOGGER_BASKET_CREATED, LOGGER_BASKET_CREATION_FAILED, diff --git a/ecommerce/extensions/iap/api/v1/views.py b/ecommerce/extensions/iap/api/v1/views.py index a45feeb06f2..fb7e5261fca 100644 --- a/ecommerce/extensions/iap/api/v1/views.py +++ b/ecommerce/extensions/iap/api/v1/views.py @@ -42,7 +42,6 @@ ERROR_ORDER_NOT_FOUND_FOR_REFUND, ERROR_REFUND_NOT_COMPLETED, ERROR_TRANSACTION_NOT_FOUND_FOR_REFUND, - ERROR_WHILE_OBTAINING_BASKET_FOR_USER, GOOGLE_PUBLISHER_API_SCOPE, LOGGER_BASKET_ALREADY_PURCHASED, LOGGER_BASKET_CREATED,