PHP Warning: Undefined array key "new" in v8.0.1

[brnbs]

Describe the bug

After updating to WC Subscriptions 7.2.1 (Core 8.0.1) the following error message appears in the server log multiple times:
[php:warn] [pid 45:tid 45] [client 127.0.0.1:36334] PHP Warning: Undefined array key "new" in /var/www/web/app/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wcs-object-data-cache-manager-many-to-one.php on line 36

woocommerce-subscriptions-core/includes/class-wcs-object-data-cache-manager-many-to-one.php

Line 36 in c1a0d3d

$this->trigger_update_cache_hook( $change['type'], $object->get_id(), $key, $change['new'], $previous_value );

To Reproduce

I just updated the plugin from 6.7.0 and enabled HPOS on the store. Not sure if the HPOS change is relevant.

Expected behavior

No warning message in the logs.

Actual behavior

I don't see any change in the actual behavior, but it might affect performance due to caching.

Product impact

• Does this issue affect WooCommerce Subscriptions? yes/no/tbc, add issue ref
• Does this issue affect WooCommerce Payments? yes/no/tbc, add issue ref

Additional context

The log contains the requester IP address: [client 127.0.0.1:36334]. Therefore, I believe this warning occurs during a cron execution.

[james-allan]

Thanks for filing this issue @brnbs. I haven't had a chance to replicate it yet, however based on the error message I think I know what's happening.

The trigger_update_cache_hook_from_change() function is only called from one place. Here: https://github.com/automattic/woocommerce-subscriptions-core/blob/8.0.1/includes/class-wcs-object-data-cache-manager.php#L205

You'll notice that the third param is $change it comes from the $this->object_changes array which is generated in the WCS_Object_Data_Cache_Manager::prepare_object_changes() function. There is only 1 case where the new array index isn't set -- the last one.

Based on that I think this error occurs when a related object cache is deleted.

Given it's the WCS_Object_Data_Cache_Manager_Many_To_One triggering this error, I suspect it's the customer's subscription cache involved.

The fix (maybe) 🛠

In short I think we need to make a change to the WCS_Object_Data_Cache_Manager_Many_To_One::trigger_update_cache_hook_from_change() function to make sure it handles meta deletes correctly. We can look at the WCS_Object_Data_Cache_Manager::trigger_update_cache_hook_from_change() for inspiration as it handles the different change types independently.

How to replicate this?

I think you'd have to delete the _customer_user/customer_id meta on a subscription. That probably explains why we haven't seen other reports of this. Deleting a subscription customer meta is not supposed to really happen.

[brnbs]

Thanks for the response @james-allan, we had some caching issues previously and got a suggestion here: #707 (comment)

One thing to add: in the meantime, we switched from Redis-based cache to Automattic/wp-cache-memcached but we observed similar strange caching issues, so we are still using the 'wcs_force_read_renewal_order_data' hook recommended by @mattallan

By the way, I’m uncertain about the recommended (most compatible) object caching solution for WooCommerce/WC Subscriptions. Any insights would be appreciated.

[james-allan]

@brnbs Just following up that I haven't been able to replicate by deleting a subscription user.

Do you have a full stack trace of the error, that might help explain the flow that leads to it. If not, it's probably still worth us defending against this error by making sure we have the new key here before referencing it.

We can do something similar to trigger_update_cache_hook_from_change() and call trigger the hook with trigger_update_cache_hook( $change['type'], $object->get_id(), $key, $change['previous'] ) when we don't have a 'new' value (ie delete).

Given I cannot replicate I'm going to give this a low priority for now.

[mattallan]

Found this similar to this one: #502