woocommerce / woocommerce-gateway-stripe

The official Stripe Payment Gateway for WooCommerce
https://wordpress.org/plugins/woocommerce-gateway-stripe/
236 stars 206 forks source link

Prevent multiple `$order->payment_complete()` calls and duplicate "Stripe charge completed" order notes during subscription renewal processing #3568

Closed mattallan closed 2 weeks ago

mattallan commented 3 weeks ago

Describe the bug

[!IMPORTANT] Customers are not being double charged as a result of this issue. There are however other potential areas of your store that could be impacted by this issue due to the multiple $order->payment_complete() calls and the multiple status transition hooks being triggered.

While processing subscription renewal payments, we are unlocking the order too early in the process leaving a small window for the payment_intent.succeeded webhook sent by Stripe to validate and proceed to mark the order as payment complete in parallel with the current request.

Here's a quick breakdown of our process_subscription_payment() function:

1. process_subscription_payment( $amount, $renewal_order )
2. lock_order_payment( $renewal_order )
3. create_and_confirm_intent_for_off_session( ... )
4. unlock_order_payment( $renewal_order )
5. get_latest_charge_from_intent()                // sends a "GET charges/ID" request off to Stripe
6. process_response( $charge, $renewal_order )    // marks the order from pending -> processing/completed status

From above, if there are delays in fetching the latest charge from Stripe, this gives time for the payment_intent.succeeded to be sent and because the order is still pending, this results in both requests eventually marking the order as payment_complete().

To Reproduce

To replicate this consistently, I had to add a small sleep( 2 ); between fetching the latest charge from Stripe and calling process_response() (here). While it seems hacky, this change in a sense just simulates a slow Stripe request to fetch a charge object which can happen.

$latest_charge = $this->get_latest_charge_from_intent( $response );
sleep( 2 );
$this->process_response( ( ! empty( $latest_charge ) ) ? $latest_charge : $response, $renewal_order );
  1. Activate WooCommerce Subscriptions and purchase a subscription product with any successful card 4242424242424242
  2. Process a renewal order using the "Process renewal" action on the Edit Subscription page
  3. While visiting admin Edit Order page for new renewal order and the admin Edit Subscription page, notice the double order notes:
Renewal Order Subscription
Image Image
contemplate commented 2 weeks ago

@mattallan We are experiencing what I believe is this bug on a client's WooCommerce Subscription site which has Object Caching Pro on Nexcess WooCommerce Hosting. We noticed that since October 15th we've had 410 Virtual Renewal Orders changed to "Processing" instead of "Completed". This is most likely due to the the integration with Autocomplete WooCommerce Orders which seems to mark the first event as "Complete" but the second one stays as "Processing". This is happening on a majority of renewal orders but not all.

I think it may be related to this issue as well but these are paid and not manual orders: https://github.com/Automattic/woocommerce-subscriptions-core/issues/707

Image

Image

WooCommerce Stripe Gateway Version 8.8.1 WooCommerce Subscriptions Version 6.8.0 WooCommerce Version 9.3.3 Autocomplete WooCommerce Orders Version 3.3.6 Object Cache Pro Version 1.21.3

I'm going to check other clients in this scenario.

georgejipa commented 2 weeks ago

One of my clients' websites is experiencing the same issue. Hopefully, a patch will be released soon.

Renewal Order Subscription
Image Image

Plugins:

WooCommerce Stripe Gateway Version 8.8.0 WooCommerce Subscriptions Version 6.8.0 WooCommerce Version 9.3.3

No object cache plugins are installed.