Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
80 stars 29 forks source link

Infinite loop when updating subscription during status change #549

Open mikkamp opened 6 months ago

mikkamp commented 6 months ago

Describe the bug

When a status transition is handled it calls several hooks where it passes the Subscription object. However the status_transition variable is not cleared until after these hooks are called. So if we hook into one of these hooks and then update/save the Subscription object it will detect that it's still in the process of transitioning status, calling the hook again.

This scenario was discovered with AutomateWoo where it's possible to create an infinite loop as is described here https://github.com/woocommerce/automatewoo/issues/1645

I can resolve the loop by calling $subscription->status_transition = false since it's a public parameter, but initially I'm wondering why the status was reset later, which is different then WooCommerce does for orders where the status is reset early. Because of this I can't reproduce the same scenario with just orders.

It seems it was moved in this commit but I don't seem to find any context why that was done: https://github.com/Automattic/woocommerce-subscriptions-core/commit/3978af01d4c2d4874ba9ab7207b93e4d6b51d0c6

To Reproduce

  1. Add an action to update subscription meta when an order note is added:
    add_action(
    'woocommerce_order_note_added',
    function ( $comment_id, $subscription ) {
        $subscription->update_meta_data( 'order_note_added', 'yes' );
        $subscription->save();
    },
    15,
    2
    );
  2. Go to a Subscription and change the status which will trigger a loop of adding order note status changes, or alternatively trigger it with a bit of code to watch it run in a debugger:
    $subscription = wcs_get_subscription( 81 );
    $subscription->set_status( 'active' === $subscription->get_status() ? 'pending' : 'active' );
    $subscription->save();

Expected behavior

For the status transition to be handled only once even if I update/save the subscription object later down the line.

Actual behavior

The status transition is not reset early enough, causing it to be repeated.

Additional context

Discovered in AutomateWoo but reproducible with any code that hooks into a status change hook or a hook that the note was added.