From 80fe28588e669d8810ec97ea040f3e211280cf53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Floure?= Date: Thu, 5 Mar 2020 11:03:47 +0100 Subject: [PATCH] Revamp stripe error handling --- uncloud/uncloud_pay/models.py | 37 +++++++----- uncloud/uncloud_pay/stripe.py | 46 ++++++-------- uncloud/uncloud_pay/templates/error.html.j2 | 18 ++++++ uncloud/uncloud_pay/views.py | 66 +++++++++++++++------ 4 files changed, 104 insertions(+), 63 deletions(-) create mode 100644 uncloud/uncloud_pay/templates/error.html.j2 diff --git a/uncloud/uncloud_pay/models.py b/uncloud/uncloud_pay/models.py index 1650e22..20e357a 100644 --- a/uncloud/uncloud_pay/models.py +++ b/uncloud/uncloud_pay/models.py @@ -139,35 +139,40 @@ class PaymentMethod(models.Model): return False def charge(self, amount): - if amount > 0: # Make sure we don't charge negative amount by errors... - if self.source == 'stripe': - stripe_customer = StripeCustomer.objects.get(owner=self.owner).stripe_id - charge_request = uncloud_pay.stripe.charge_customer( - amount, stripe_customer, self.stripe_payment_method_id) - if charge_request['error'] == None: - payment = Payment(owner=self.owner, source=self.source, amount=amount) - payment.save() # TODO: Check return status + if not self.active: + raise Exception('This payment method is inactive.') - return payment - else: - raise Exception('Stripe error: {}'.format(charge_request['error'])) - else: - raise Exception('This payment method is unsupported/cannot be charged.') - else: + 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 + stripe_payment = uncloud_pay.stripe.charge_customer( + amount, stripe_customer, self.stripe_payment_method_id) + 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_payment['error']) + else: + raise Exception('This payment method is unsupported/cannot be charged.') + 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 ### diff --git a/uncloud/uncloud_pay/stripe.py b/uncloud/uncloud_pay/stripe.py index 1d745ef..7dc53c6 100644 --- a/uncloud/uncloud_pay/stripe.py +++ b/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 = { - 'response_object': response_object, - 'error': None - } - return response + return response_object 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( - {'error': "Too many requests made to the API too quickly"}) - return response + logging.error("Too many requests made to the API too quickly.") + raise Exception(common_message) except stripe.error.InvalidRequestError as e: logging.error(str(e)) - response.update({'error': "Invalid parameters"}) - return response + raise Exception('Invalid parameters.') 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}) - return response + raise Exception(common_message) except stripe.error.APIConnectionError as e: logging.error(str(e)) - response.update({'error': common_message}) - return response + raise Exception(common_message) except stripe.error.StripeError as e: - # maybe send email + # XXX: maybe send email logging.error(str(e)) - response.update({'error': common_message}) - return response + raise Exception(common_message) except Exception as e: # maybe send email logging.error(str(e)) - response.update({'error': common_message}) - return response + raise Exception(common_message) 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) - if customer_request['error'] == None: - mapping = uncloud_pay.models.StripeCustomer.objects.create( + try: + customer = create_customer(user.username, user.email) + uncloud_stripe_mapping = uncloud_pay.models.StripeCustomer.objects.create( owner=user, stripe_id=customer_request['response_object']['id'] ) - return mapping.stripe_id - else: + return uncloud_stripe_mapping.stripe_id + except Exception as e: return None @handle_stripe_error diff --git a/uncloud/uncloud_pay/templates/error.html.j2 b/uncloud/uncloud_pay/templates/error.html.j2 new file mode 100644 index 0000000..ba9209c --- /dev/null +++ b/uncloud/uncloud_pay/templates/error.html.j2 @@ -0,0 +1,18 @@ + + + + Error + + + +
+

Error

+

{{ error }}

+
+ + diff --git a/uncloud/uncloud_pay/views.py b/uncloud/uncloud_pay/views.py index a22c616..08e94a0 100644 --- a/uncloud/uncloud_pay/views.py +++ b/uncloud/uncloud_pay/views.py @@ -58,25 +58,28 @@ class PaymentMethodViewSet(viewsets.ModelViewSet): if serializer.validated_data['source'] == "stripe": # Retrieve Stripe customer ID for user. - customer_id = uncloud_stripe.get_customer_id_for(request.user) + customer_id = uncloud_stripe.get_customer_id_for(request.user) if customer_id == None: return Response( {'error': 'Could not resolve customer stripe ID.'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) - # TODO: handle error - setup_intent = uncloud_stripe.create_setup_intent(customer_id) + 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( - payment_method.uuid - ) - stripe_registration_url = reverse('api-root', request=request) + query + path = "payment-method/{}/register-stripe-cc".format( + payment_method.uuid) + stripe_registration_url = reverse('api-root', request=request) + path 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() - setup_intent = uncloud_stripe.get_setup_intent( - payment_method.stripe_setup_intent_id) + 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"], - 'stripe_pk': uncloud_stripe.public_api_key + 'client_secret': setup_intent.client_secret, + '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') - def register_stripe_cc(self, request, pk=None): + @action(detail=True, methods=['post'], url_path='activate-stripe-cc') + def activate_stripe_cc(self, request, pk=None): payment_method = self.get_object() - setup_intent = uncloud_stripe.get_setup_intent( - payment_method.stripe_setup_intent_id) + 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 - if payment_method_id: - payment_method.stripe_payment_method_id = payment_method_id + print(setup_intent) + if setup_intent.payment_method: + payment_method.stripe_payment_method_id = setup_intent.payment_method payment_method.save() return Response({