Revamp stripe error handling

uncloud/uncloud_pay/models.py

@@ -139,35 +139,40 @@ class PaymentMethod(models.Model):
             return False
 
     def charge(self, amount):
+        if not self.active:
+            raise Exception('This payment method is inactive.')
+
         if amount > 0: # Make sure we don't charge negative amount by errors...
+            raise Exception('Cannot charge negative amount.')
+
         if self.source == 'stripe':
             stripe_customer = StripeCustomer.objects.get(owner=self.owner).stripe_id
-                charge_request = uncloud_pay.stripe.charge_customer(
+            stripe_payment = uncloud_pay.stripe.charge_customer(
                     amount, stripe_customer, self.stripe_payment_method_id)
-                if charge_request['error'] == None:
+            if stripe_payment['paid']:
                 payment = Payment(owner=self.owner, source=self.source, amount=amount)
                 payment.save() # TODO: Check return status
 
                 return payment
             else:
-                    raise Exception('Stripe error: {}'.format(charge_request['error']))
+                raise Exception(stripe_payment['error'])
         else:
             raise Exception('This payment method is unsupported/cannot be charged.')
-        else:
-            raise Exception('Cannot charge negative amount.')
 
 
     def get_primary_for(user):
         methods = PaymentMethod.objects.filter(owner=user)
         for method in methods:
             # Do we want to do something with non-primary method?
-            if method.primary:
+            if method.active and method.primary:
                 return method
 
         return None
 
     class Meta:
-        #API_keyunique_together = [['owner', 'primary']]
+        # TODO: limit to one primary method per user.
+        # unique_together is no good since it won't allow more than one
+        # non-primary method.
         pass
 
 ###

uncloud/uncloud_pay/stripe.py

@@ -17,6 +17,7 @@ CURRENCY = 'chf'
 stripe.api_key = uncloud.secrets.STRIPE_KEY
 
 # Helper (decorator) used to catch errors raised by stripe logic.
+# Catch errors that should not be displayed to the end user, raise again.
 def handle_stripe_error(f):
     def handle_problems(*args, **kwargs):
         response = {
@@ -25,49 +26,38 @@ def handle_stripe_error(f):
             'error': None
         }
 
-        common_message = "Currently it is not possible to make payments."
+        common_message = "Currently it is not possible to make payments. Please try agin later."
         try:
             response_object = f(*args, **kwargs)
-            response = {
+            return response_object
-                'response_object': response_object,
-                'error': None
-            }
-            return response
         except stripe.error.CardError as e:
             # Since it's a decline, stripe.error.CardError will be caught
             body = e.json_body
-            err = body['error']
-            response.update({'error': err['message']})
             logging.error(str(e))
-            return response
+
+            raise e # For error handling.
         except stripe.error.RateLimitError:
-            response.update(
+            logging.error("Too many requests made to the API too quickly.")
-                {'error': "Too many requests made to the API too quickly"})
+            raise Exception(common_message)
-            return response
         except stripe.error.InvalidRequestError as e:
             logging.error(str(e))
-            response.update({'error': "Invalid parameters"})
+            raise Exception('Invalid parameters.')
-            return response
         except stripe.error.AuthenticationError as e:
             # Authentication with Stripe's API failed
             # (maybe you changed API keys recently)
             logging.error(str(e))
-            response.update({'error': common_message})
+            raise Exception(common_message)
-            return response
         except stripe.error.APIConnectionError as e:
             logging.error(str(e))
-            response.update({'error': common_message})
+            raise Exception(common_message)
-            return response
         except stripe.error.StripeError as e:
-            # maybe send email
+            # XXX: maybe send email
             logging.error(str(e))
-            response.update({'error': common_message})
+            raise Exception(common_message)
-            return response
         except Exception as e:
             # maybe send email
             logging.error(str(e))
-            response.update({'error': common_message})
+            raise Exception(common_message)
-            return response
 
     return handle_problems
 
@@ -82,14 +72,14 @@ def get_customer_id_for(user):
         return uncloud_pay.models.StripeCustomer.objects.get(owner=user).stripe_id
     except ObjectDoesNotExist:
         # No entry yet - making a new one.
-        customer_request = create_customer(user.username, user.email)
+        try:
-        if customer_request['error'] == None:
+            customer = create_customer(user.username, user.email)
-            mapping = uncloud_pay.models.StripeCustomer.objects.create(
+            uncloud_stripe_mapping = uncloud_pay.models.StripeCustomer.objects.create(
                     owner=user,
                     stripe_id=customer_request['response_object']['id']
                     )
-            return mapping.stripe_id
+            return uncloud_stripe_mapping.stripe_id
-        else:
+        except Exception as e:
             return None
 
 @handle_stripe_error

uncloud/uncloud_pay/templates/error.html.j2

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+	<head>
+		<title>Error</title>
+		<style>
+			#content {
+				width: 400px;
+				margin: auto;
+			}
+		</style>
+	</head>
+	<body>
+		<div id="content">
+			<h1>Error</h1>
+			<p>{{ error }}</p>
+		</div>
+	</body>
+</html>

uncloud/uncloud_pay/views.py

@@ -64,19 +64,22 @@ class PaymentMethodViewSet(viewsets.ModelViewSet):
                     {'error': 'Could not resolve customer stripe ID.'},
                     status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
-            # TODO: handle error
+            try:
                 setup_intent = uncloud_stripe.create_setup_intent(customer_id)
+            except Exception as e:
+                return Response({'error': str(e)},
+                    status=status.HTTP_500_INTERNAL_SERVER_ERROR)
+
             payment_method = PaymentMethod.objects.create(
                     owner=request.user,
-                    stripe_setup_intent_id=setup_intent['response_object']['id'],
+                    stripe_setup_intent_id=setup_intent.id,
                     **serializer.validated_data)
 
             # TODO: find a way to use reverse properly:
             # https://www.django-rest-framework.org/api-guide/reverse/
-            query= "payment-method/{}/register-stripe-cc".format(
+            path = "payment-method/{}/register-stripe-cc".format(
-                    payment_method.uuid
+                    payment_method.uuid)
-                    )
+            stripe_registration_url = reverse('api-root', request=request) + path
-            stripe_registration_url = reverse('api-root', request=request) + query
             return Response({'please_visit': stripe_registration_url})
 
         return Response(serializer.data)
@@ -97,26 +100,51 @@ class PaymentMethodViewSet(viewsets.ModelViewSet):
     @action(detail=True, methods=['get'], url_path='register-stripe-cc', renderer_classes=[TemplateHTMLRenderer])
     def register_stripe_cc(self, request, pk=None):
         payment_method = self.get_object()
+        if payment_method.source != 'stripe':
+            return Response(
+                    {'error': 'This is not a Stripe-based payment method.'},
+                    template_name='error.html.j2')
+
+        if payment_method.active:
+            return Response(
+                    {'error': 'This payment method is already active'},
+                    template_name='error.html.j2')
+
+        try:
             setup_intent = uncloud_stripe.get_setup_intent(
                     payment_method.stripe_setup_intent_id)
+        except Exception as e:
+            return Response(
+                    {'error': str(e)},
+                    template_name='error.html.j2')
+
+        # TODO: find a way to use reverse properly:
+        # https://www.django-rest-framework.org/api-guide/reverse/
+        callback_path= "payment-method/{}/activate-stripe-cc/".format(
+                payment_method.uuid)
+        callback = reverse('api-root', request=request) + callback_path
 
         # Render stripe card registration form.
         template_args = {
-                'client_secret': setup_intent["response_object"]["client_secret"],
+                'client_secret': setup_intent.client_secret,
-                'stripe_pk': uncloud_stripe.public_api_key
+                'stripe_pk': uncloud_stripe.public_api_key,
+                'callback': callback
                 }
         return Response(template_args, template_name='stripe-payment.html.j2')
 
-    @action(detail=True, methods=['post'], url_path='register-stripe-cc')
+    @action(detail=True, methods=['post'], url_path='activate-stripe-cc')
-    def register_stripe_cc(self, request, pk=None):
+    def activate_stripe_cc(self, request, pk=None):
         payment_method = self.get_object()
+        try:
             setup_intent = uncloud_stripe.get_setup_intent(
                     payment_method.stripe_setup_intent_id)
+        except Exception as e:
+            return Response({'error': str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
         # Card had been registered, fetching payment method.
-        payment_method_id = setup_intent["response_object"].payment_method
+        print(setup_intent)
-        if payment_method_id:
+        if setup_intent.payment_method:
-            payment_method.stripe_payment_method_id = payment_method_id
+            payment_method.stripe_payment_method_id = setup_intent.payment_method
             payment_method.save()
 
             return Response({