Automattic / woocommerce-services

WooCommerce Services is a feature plugin that integrates hosted services into WooCommerce (3.0+), and currently includes automated tax rates and the ability to purchase and print USPS shipping labels.
GNU General Public License v2.0
107 stars 20 forks source link

PHP notices with WP Debug enabled #2653

Closed waclawjacek closed 1 year ago

waclawjacek commented 1 year ago

Reported in p1683027614033619-slack-C7U3Y3VMY

Merchant has WP Debug on which is causing some issues during the checkout due to notices being output by other plugins.

During troubleshooting, we found out that the notices are being output by WooCommerce Shipping & Tax

Invalid argument supplied for foreach() in /home/redacted/public_html/redacted/wp-content/plugins/woocommerce-services/woocommerce-services.php on line 1115

Trying to access array offset on value of type bool in /home/redacted/public_html/redacted/wp-content/plugins/woocommerce-services/woocommerce-services.php on line 1112

Do we need to check packages before accessing them in woocommerce-services.php? This occurs after placing an order / pressing checkout button. So hooked in woocommerce_checkout_order_processed.

johndcoy commented 1 year ago

@waclawjacek I was not able to replicate this issue. Do you have the steps to replicate the problem?

waclawjacek commented 1 year ago

Hi @johndcoy! I gave the code a closer look and it looks like this is the relevant bit of code in woocommerce-services.php:

https://github.com/Automattic/woocommerce-services/blob/689a1f1c24296bd97011d0e0e7fc88d543e32e17/woocommerce-services.php#L1111-L1115

On line 1111, the calculate_shipping_for_package call is actually a call on something from WC Core, and it is doing this:

    /**
     * Calculate shipping rates for a package,
     *
     * Calculates each shipping methods cost. Rates are stored in the session based on the package hash to avoid re-calculation every page load.
     *
     * @param array $package Package of cart items.
     * @param int   $package_key Index of the package being calculated. Used to cache multiple package rates.
     *
     * @return array|bool
     */
    public function calculate_shipping_for_package( $package = array(), $package_key = 0 ) {
        // If shipping is disabled or the package is invalid, return false.
        if ( ! $this->enabled || empty( $package ) ) {
            return false;
        }

        $package['rates'] = array();

Meaning, this method can sometimes return false instead of an array with the "rates" index which WCS&T should support (especially since the method's docblock says it does it) but it doesn't. We should add support for it.

iyut commented 1 year ago

A little update : I have fixed the issue in this PR. But still cannot merge it because some tests are failing. I'm still trying to check what's going on at the moment.

waclawjacek commented 1 year ago

@iyut, @samnajian has encountered this recently as well. You could try getting in touch with him to discuss this!

iyut commented 1 year ago

@waclawjacek thanks for the info! I'll try to get in touch with him.

kaushikasomaiya commented 1 year ago

7091741-zen

74MR47 commented 1 year ago

Another report:

ZD: 7086701

Forum Thread: 👉 invalid argument in woocommerce-services.php