New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Card.api_retrieve() will crash if the customer associated with the card has been deleted #432

Closed
jleclanche opened this Issue Feb 28, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@jleclanche
Contributor

jleclanche commented Feb 28, 2017

Situation: Calling on_delete_subscriber_purge_customer on a customer that has already been deleted by Stripe.

Eventually, we enter Customer.purge() which hits for source in self.sources.all(): source.remove().

However, if the customer has already been deleted, calling remove() on a source whose customer has already been deleted will crash, because it first calls retrieve() on the card object. This is caused by the StripeCard.api_retrieve method:

    def api_retrieve(self, api_key=settings.STRIPE_SECRET_KEY):
        # OVERRIDING the parent version of this function [...]
        return self.customer.api_retrieve(api_key=api_key).sources.retrieve(self.stripe_id, expand=self.expand_fields)

Now, what happens if self.customer.api_retrieve(...) returns a deleted customer? See for yourself:

>>> customer = Customer.retrieve(id="cus_ACOAhvtRVLJTDT", api_key=settings.STRIPE_SECRET_KEY)
>>> customer
<Customer id=cus_ACOAhvtRVLJTDT at 0x7fd435fffae8> JSON: {
  "deleted": true,
  "id": "cus_ACOAhvtRVLJTDT"
}
>>> "sources" in customer
False
>>> customer.sources.retrieve(...)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/vagrant/env/lib/python3.4/site-packages/stripe/resource.py", line 135, in __getattr__
    raise AttributeError(*err.args)
AttributeError: sources
@jleclanche

This comment has been minimized.

Show comment
Hide comment
@jleclanche

jleclanche Feb 28, 2017

Contributor

This is pretty hard to fix without reworking the djstripe api because api_retrieve() is always expected to either return a real object, or a native stripe error.

We can check for if "sources" not in customer... but then what to do with that? raise our own error? return none? neither of those are handled anywhere.

Contributor

jleclanche commented Feb 28, 2017

This is pretty hard to fix without reworking the djstripe api because api_retrieve() is always expected to either return a real object, or a native stripe error.

We can check for if "sources" not in customer... but then what to do with that? raise our own error? return none? neither of those are handled anywhere.

@jleclanche

This comment has been minimized.

Show comment
Hide comment
@jleclanche

jleclanche Feb 28, 2017

Contributor

Something like this should work, but ain't pretty:

diff --git a/djstripe/stripe_objects.py b/djstripe/stripe_objects.py
index 600bfd8..161b0bb 100644
--- a/djstripe/stripe_objects.py
+++ b/djstripe/stripe_objects.py
@@ -1199,10 +1199,13 @@ Fields not implemented:
     def api_retrieve(self, api_key=settings.STRIPE_SECRET_KEY):
         # OVERRIDING the parent version of this function
         # Cards must be manipulated through a customer or account.
-        # TODO: When managed accounts are supported, this method needs to check if either a customer or
-        #       account is supplied to determine the correct object to use.
-
-        return self.customer.api_retrieve(api_key=api_key).sources.retrieve(self.stripe_id, expand=self.expand_fields)
+        # TODO: When managed accounts are supported, this method needs to check if
+        # either a customer or account is supplied to determine the correct object to use.
+        customer = self.customer.api_retrieve(api_key=api_key)
+        if "sources" not in customer:
+            # The customer has most likely been deleted and doesn't have a sources attribute
+            raise InvalidRequestError("No such source: %s" % (self.stripe_id))
+        return customer.sources.retrieve(self.stripe_id, expand=self.expand_fields)
 
     @staticmethod
     def _get_customer_from_kwargs(**kwargs):
Contributor

jleclanche commented Feb 28, 2017

Something like this should work, but ain't pretty:

diff --git a/djstripe/stripe_objects.py b/djstripe/stripe_objects.py
index 600bfd8..161b0bb 100644
--- a/djstripe/stripe_objects.py
+++ b/djstripe/stripe_objects.py
@@ -1199,10 +1199,13 @@ Fields not implemented:
     def api_retrieve(self, api_key=settings.STRIPE_SECRET_KEY):
         # OVERRIDING the parent version of this function
         # Cards must be manipulated through a customer or account.
-        # TODO: When managed accounts are supported, this method needs to check if either a customer or
-        #       account is supplied to determine the correct object to use.
-
-        return self.customer.api_retrieve(api_key=api_key).sources.retrieve(self.stripe_id, expand=self.expand_fields)
+        # TODO: When managed accounts are supported, this method needs to check if
+        # either a customer or account is supplied to determine the correct object to use.
+        customer = self.customer.api_retrieve(api_key=api_key)
+        if "sources" not in customer:
+            # The customer has most likely been deleted and doesn't have a sources attribute
+            raise InvalidRequestError("No such source: %s" % (self.stripe_id))
+        return customer.sources.retrieve(self.stripe_id, expand=self.expand_fields)
 
     @staticmethod
     def _get_customer_from_kwargs(**kwargs):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment