From 525d363b560ca5b3671850163ecd0ec9e6c8a290 Mon Sep 17 00:00:00 2001
From: Damien Bouvy <dbo@odoo.com>
Date: Tue, 31 Dec 2019 09:18:54 +0000
Subject: [PATCH] [FIX] payment_stripe: better error handling

In some cases, Stripe will return the result of a transaction with
a faulty HTTP Status (e.g. 4XX statuses) - not because the request
was malformed, but because the payment failed. It is a bit unfortunate
that Stripe would not differentiate between payment status and request
validity, but that's the state of things.

This commit ensures that such a response will be logged correctly, by
making the call to `raise_for_status` conditionnal on the response object's
status code and internal structure, forwarding the response's json to the
validation flow if the status is in the 4XX range and a `code` key is
found in the response, as described in https://stripe.com/docs/error-codes

While testing this fix, it became apparent that some error message
processing in the frontend was not correctly handled as well - instead
of purely rejecting the promise, a failed payment should still send its
payload to the backend, to allow the server to put the transaction in
the correct state according to the response.
---
 addons/payment_stripe/models/payment.py | 31 ++++++++++++-------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/addons/payment_stripe/models/payment.py b/addons/payment_stripe/models/payment.py
index 39a8a67493e..f7aeeb770a1 100644
--- a/addons/payment_stripe/models/payment.py
+++ b/addons/payment_stripe/models/payment.py
@@ -61,17 +61,19 @@ class PaymentAcquirerStripe(models.Model):
             'Stripe-Version': '2019-05-16', # SetupIntent need a specific version
             }
         resp = requests.request(method, url, data=data, headers=headers)
-        try:
-            resp.raise_for_status()
-        except HTTPError:
-            _logger.error("Here are some more information for the "
-                          "following HTTP error on %s:\n"
-                          "Request data:\n%s\n"
-                          "Response body:\n%s",
-                          url, pprint.pformat(data), resp.text)
-            stripe_error = resp.json().get('error', {}).get('message', '')
-            error_msg = " " + (_("Stripe gave us the following info about the problem: '%s'") % stripe_error)
-            raise ValidationError(error_msg)
+        # Stripe can send 4XX errors for payment failure (not badly-formed requests)
+        # check if error `code` is present in 4XX response and raise only if not
+        # cfr https://stripe.com/docs/error-codes
+        # these can be made customer-facing, as they usually indicate a problem with the payment
+        # (e.g. insufficient funds, expired card, etc.)
+        if not resp.ok and not (400 <= resp.status_code < 500 and resp.json().get('error', {}).get('code')):
+            try:
+                resp.raise_for_status()
+            except HTTPError:
+                _logger.error(resp.text)
+                stripe_error = resp.json().get('error', {}).get('message', '')
+                error_msg = " " + (_("Stripe gave us the following info about the problem: '%s'") % stripe_error)
+                raise ValidationError(error_msg)
         return resp.json()
 
     def _create_stripe_session(self, kwargs):
@@ -276,11 +278,8 @@ class PaymentTransactionStripe(models.Model):
             self._set_transaction_pending()
             return True
         else:
-            error = tree.get('failure_message')
-            _logger.warning(error)
-            vals.update({'state_message': error})
-            self.write(vals)
-            self._set_transaction_cancel()
+            error = tree.get("failure_message") or tree.get('error', {}).get('message')
+            self._set_transaction_error(error)
             return False
 
     def _stripe_form_get_invalid_parameters(self, data):