refactor: Improve exception handling for mobile IAP

ecommerce/extensions/iap/api/v1/constants.py

@@ -12,16 +12,14 @@
 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]"
 LOGGER_BASKET_CREATION_FAILED = "Basket creation failed for user [%s]. Error: [%s]"
 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]. " \

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
@@ -36,13 +36,12 @@
     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,
     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 +321,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 +458,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 +574,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

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
@@ -42,14 +42,13 @@
     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,
     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 +223,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 +248,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)