Skip to content
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

Failed charge webhooks #317

Merged
merged 3 commits into from
Mar 16, 2017
Merged

Failed charge webhooks #317

merged 3 commits into from
Mar 16, 2017

Conversation

meshy
Copy link
Contributor

@meshy meshy commented Feb 27, 2017

When dealing with the webhook event after using the stripe test card 4000 0000 0000 0341, I was getting the following error:

Click here to show/hide stacktrace
Internal Server Error: /payments/webhook/
Traceback (most recent call last):
  File "/.../django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File "/.../channels/handler.py", line 228, in process_exception_by_middleware
    return super(AsgiHandler, self).process_exception_by_middleware(exception, request)
  File "/.../django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib64/python3.4/contextlib.py", line 30, in inner
    return func(*args, **kwds)
  File "/.../django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/.../django/utils/decorators.py", line 67, in _wrapper
    return bound_func(*args, **kwargs)
  File "/.../django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/.../django/utils/decorators.py", line 63, in bound_func
    return func.__get__(self, type(self))(*args2, **kwargs2)
  File "/.../pinax/stripe/views.py", line 185, in dispatch
    return super(Webhook, self).dispatch(*args, **kwargs)
  File "/.../django/views/generic/base.py", line 88, in dispatch
    return handler(request, *args, **kwargs)
  File "/.../pinax/stripe/views.py", line 200, in post
    message=data
  File "/.../pinax/stripe/actions/events.py", line 30, in add_event
    webhook.process()
  File "/.../pinax/stripe/webhooks.py", line 91, in process
    self.process_webhook()
  File "/.../pinax/stripe/webhooks.py", line 171, in process_webhook
    stripe.Charge.retrieve(self.event.message["data"]["object"]["id"])
  File "/.../pinax/stripe/actions/charges.py", line 99, in sync_charge_from_stripe_data
    customer = models.Customer.objects.get(stripe_id=data["customer"])
  File "/.../django/db/models/manager.py", line 122, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/.../django/db/models/query.py", line 387, in get
    self.model._meta.object_name
pinax.stripe.models.DoesNotExist: Customer matching query does not exist.

This is because the json data returned by the webook contains "customer": null,.

This PR introduces a check to make sure the customer ID is in the JSON data before attempting to get a Customer based on it.

Currently this fails because Charge.customer is not a nullable field. That appears to have already been addressed in #307.

This can happen when `actions.customers.create` is called with a token
for a card that gets rejected by the bank.

To reproduce this, use the stripe card number 4000 0000 0000 0341.
This can happen when a card token is declined by the bank.

Try stripe card number 4000 0000 0000 0341.

This currently fails because Charge.customer is a required field. There
is another PR (#307) to address that issue.
@meshy meshy changed the title Failed charges Failed charge webhooks Feb 27, 2017
if customer_id:
customer = models.Customer.objects.get(stripe_id=customer_id)
else:
customer = None
Copy link
Contributor Author

@meshy meshy Feb 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just noticed that #307 takes a different approach to this.

The code is shorter, but does a lookup that might not be required.

@meshy
Copy link
Contributor Author

meshy commented Mar 16, 2017

@paltman thank you for the flurry of merges! As this PR is now reduced to a single test case that (due to your merges) now passes, I would understand if you would like to close this without merging -- I'll leave it open for the moment, and in your hands.

@paltman paltman merged commit 53bdee3 into pinax:master Mar 16, 2017
@paltman
Copy link
Contributor

paltman commented Mar 16, 2017

More tests the better!

Thanks!

@meshy meshy deleted the failed-charges branch March 16, 2017 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants