StripeReview pane throws uncaught InvalidRequestException on /review reload after failed charge

Thank you very much @alex.bukach. Makes a lot of sense to me. For the details I think the maintainers need to take a look, but I left two general comments for now.

Logically LGTM.

Changing the priority of this because this results in a WSOD that cannot be resolved by the customer because they will always be sent to the review step where the API call runs and fails again so this halts checkout permanently for anonymous checkout customers.

Also while it's easiest to trigger the error with card element gateway, this error can happen to the Payment Element gateway too if for any reason the API call to update the PaymentIntent fails.

Cleaned up the code, removed the unnecessary try/catch for setting the payment method to non reusable.

Tried to add logging for the intent cancel catch but I'm not sure if I can nest them like that with the $e variable so I just did a more generic error message with the intent id and order id.

I assumed that with $logger being in the class it had been setup. That was incomplete code from another commit. The service has now been setup properly.

Testing this out some, it seems like canceling the payment intent here is unnecessary as it automatically gets canceled by an event subscriber once the order is saved and information step is loaded again.

So this whole code block can go away

        try {
          $intent->cancel();
        }
        catch (\Throwable) {
          // The stale intent may already be in a non-cancellable state.
          $this->logger->warning(
            'Cancelling intent @intent_id for order @id failed',
            ['@intent_id' => $intent->id, '@id' => $this->order->id()]
          );
        }
        $this->order->unsetData('stripe_intent');

Removing the payment method from the order is definitely necessary. Setting the payment method as not reusable is not enough and it will remain attached to the order causing a different error from stripe about the payment intent missing a payment method.

Code is at a place I feel this is usable in production.

For mantainers: If you're wondering if setting the payment method to be not reusable here is a good idea consider that this code path will only ever be triggered when Stripe considers a payment method to be not reusable which as far as I know only happens when it's not attached to a customer. So this won't go and mark returning customer's payment methods as not reusable because they had insufficient funds or failed 3DS authentication. It only affects payment methods stripe already considers to be not reusable.

Thanks @rhovland that makes a lot of sense to me! Great to see this progress, I hope maintainers will have time soon to review this code soon. We'll also check out, if it solves (some of) our present issues.

I'm surprised this issue hadn't already been addressed. 1 in every 20th anonymous customer hits this bug on our site. It is not a fringe bug that gets triggered rarely.

Done a bunch of testing on these changes:

Anonymous customer fails due to 3DS/insufficient funds:

• Clicks edit next to Payment information on review pane or "Go back" button next to "Pay and complete"; tries attached payment method again; sent back to information step saying to re-enter their payment method and the one attached to the order is now gone.
• Navigates away from checkout, then back to it, being sent to review step then redirected to information step to enter new payment method with one attached to the order is now gone.
• Enters the same payment method but transaction authentication succeeds this time and order is placed.

All of the operations as a logged in customer work the same as before the changes.