Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
88 stars 33 forks source link

Fatal error if the user has an already paid renewal left in the cart #537

Closed a-danae closed 10 months ago

a-danae commented 11 months ago

Describe the bug

If a user renews their subscription successfully, but for some (unknown) reason, the same product remains in the cart, the website will stop with a fatal error for that user.

The reason behind the fatal error is the variable $item_to_renew that's conditionally defined.

Stack trace ``` [01-Nov-2023 13:41:15 UTC] PHP Warning: Undefined variable $item_to_renew in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 448 [01-Nov-2023 13:41:15 UTC] PHP Warning: Trying to access array offset on value of type null in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 448 [01-Nov-2023 13:41:15 UTC] PHP Warning: Undefined variable $item_to_renew in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 463 [01-Nov-2023 13:41:15 UTC] PHP Warning: Trying to access array offset on value of type null in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 463 [01-Nov-2023 13:41:15 UTC] PHP Warning: Trying to access array offset on value of type null in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 463 [01-Nov-2023 13:41:15 UTC] PHP Fatal error: Uncaught TypeError: array_sum(): Argument 1 ($array) must be of type array, null given in .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php:463 Stack trace: 0- .../woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php(463): array_sum() 1- .../wp-includes/class-wp-hook.php(310): WCS_Cart_Renewal->get_cart_item_from_session() 2- .../wp-includes/plugin.php(205): WP_Hook->apply_filters() 3- .../wp-content/plugins/woocommerce/includes/class-wc-cart-session.php(206): apply_filters() 4- .../wp-includes/class-wp-hook.php(310): WC_Cart_Session->get_cart_from_session() 5- .../wp-includes/class-wp-hook.php(334): WP_Hook->apply_filters() 6- .../wp-includes/plugin.php(517): WP_Hook->do_action() 7- .../wp-settings.php(654): do_action() 8- .../wp-config.php(133): require_once('...') 9- .../wp-load.php(50): require_once('...') 10- .../wp-blog-header.php(13): require_once('...') 11- .../index.php(17): require('...') 12- {main} thrown in .../wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php on line 463 ```

To Reproduce

We don't have replication steps, but the error makes sense when checking out the code.

Expected behavior

The unhappy path described by the user must not trigger a fatal error.

Actual behavior

Product impact

Additional context

Reported in 7247818-zd-a8c

piotrkunicki commented 10 months ago

We have a customer who is experiencing exactly the same issue. (I've checked error log file and stack trace is the same)

Do you know any temporary solution for this problem? Unfortunately, the customer currently has no access to our services as a fatal error occurs after logging in..

slapic commented 10 months ago

@piotrkunicki I reported this issue to the support weeks ago. There's only a temporary patch you can do in the source code to get rid of this fatal error. Woo is pretty slow to respond, and it took weeks until they accepted the report. So I don't expect any solution in the near future, unfortunately (check who is assigned to the issue: no one, no priority, no milstone, it's just a dead case).

The temporary solution is to edit the relevant file: vi public_html/wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-cart-renewal.php

Look for the line 463: $price += array_sum( $item_to_renew['taxes']['subtotal'] ); // Use the taxes array items here as they contain taxes to a more accurate number of decimals.

Change it and include the missing type check: if (is_array($item_to_renew)) $price += array_sum( $item_to_renew['taxes']['subtotal'] ); // Use the taxes array items here as they contain taxes to a more accurate number of decimals.

Note that it's a temporary solution and I don't know if it has any side effects (not for me so far). Note that any WooCommerce Subscription update will overwrite this file without a notice and you need to apply the change after each update until Woo fixes the issue.