746460 - [traceback] In-app payments returns a 500 if pre-auth is not set up

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P1)

Description

[krupa raj[:krupa]]

steps to reproduce:
1. Log into Marketplace with an account which doesn't have preauth set up
2. Go to http://inapp-pay-test.farmdev.com/en-US/ and click the call mozmarket.buy() button
3. In the purchase modal, click the 0.99 USD button


observed behavior:
In-app payments returns a 500 if pre-auth is not set up

traceback details:
Traceback (most recent call last):

 File "/data/www/addons-dev.allizom.org/zamboni/vendor/src/django/django/core/handlers/base.py", line 111, in get_response
   response = callback(request, *callback_args, **callback_kwargs)

 File "/data/www/addons-dev.allizom.org/zamboni/vendor/src/commonware/commonware/response/decorators.py", line 18, in _wrapped_view
   response = view_fn(request, *args, **kwargs)

 File "/data/www/addons-dev.allizom.org/zamboni/mkt/inapp_pay/decorators.py", line 30, in wrapper
   return view(request, signed_req, req, *args, **kw)

 File "/data/www/addons-dev.allizom.org/zamboni/apps/amo/decorators.py", line 29, in wrapper
   return func(request, *args, **kw)

 File "/data/www/addons-dev.allizom.org/zamboni/apps/amo/decorators.py", line 50, in wrapper
   return f(request, *args, **kw)

 File "/data/www/addons-dev.allizom.org/zamboni/apps/amo/decorators.py", line 112, in wrapper
   return f(*args, **kw)

 File "/data/www/addons-dev.allizom.org/zamboni/apps/amo/decorators.py", line 104, in wrapper
   return f(*args, **kw)

 File "/data/www/addons-dev.allizom.org/zamboni/vendor/src/django-waffle/waffle/decorators.py", line 36, in _wrapped_view
   return view(request, *args, **kwargs)

 File "/data/www/addons-dev.allizom.org/zamboni/mkt/inapp_pay/views.py", line 146, in pay
   'user %s (status %s)' % (request.amo_user, status))

InappPaymentError: PayPal did not recognize preauth token for user 5540398: "'><script>alert('problem')</script> (status CREATED)

Comment 1

[Kumar McMillan [:kumar]]

kicking to next week since this is a blocker

Comment 2

[Andy McKay]

Pre-auth can and will fail, so you need to handle this on the front end. It throws a 500 because @potch added that here:  

https://github.com/mozilla/zamboni/commit/2ce9ab8d#diff-16

In purchasing when it fails, I just retry not using pre-auth. What would you like to happen. Catch the 500 in the client? I would presume you'll want an error status and string returned a 200?

Comment 3

[Kumar McMillan [:kumar]]

when you say preauth can and will fail, do you mean it is an intermittent failure? Or is this something where we should say to the user "sorry, pre-auth failed, please set it up again."

Comment 4

[Andy McKay]

I have seen intermittent failures on the sandbox, but that could be the sandbox. There are two kinds of failures:

- fatal, for example user goes to the paypal account and removes the pre-auth ability, we don't know that's failed, they have to set it up again. This class of errors includes pre-auth expired, pre-auth limit reached and so on.

- less fatal, for example: the currency that the user is using is not a currency that the developer accepts or the buyer just has no money in their account. In this case there may or may not be something the end user can figure out what to do.

We've got a list of status codes back from paypal, correctly predicting and categorizing the errors is a little hard for in-app. There's at least one bug to increase the visibility of these errors, bug 747057.

Comment 5

[Kumar McMillan [:kumar]]

We used to handle this before by kicking the user over to the PayPal UI. We redirected to a PayPal URL here: https://github.com/mozilla/zamboni/commit/2ce9ab8dfd9cab7ed35e2a15e643d239dcdca2db#L16L137

Maybe we should put that redirect back if preauth fails *and* request.amo_user.get_preapproval() is True?

Comment 6

[Andy McKay]

I think that's a good first start.

Comment 7

[Andy McKay]

This was pushed and kumar added some bugs about cleaning up the UI.

https://github.com/mozilla/zamboni/commit/5c913f

Comment 8

[Victor Carciu]

Attachment: Postfix Screenshot

Comment 9

[Kumar McMillan [:kumar]]

UI cleanup will be in bug 748137