RC
/Remi Corson

The integer that delayed WooCommerce 11.0

On July 28, the WooCommerce team announced that 11.0 would not ship that day:

During early testing of WooCommerce 11.0.0 RC1, we identified a fatal error under specific circumstances from a new performance feature.

New date: August 4. That is the whole announcement, which is fair enough for a release note, but "a fatal error under specific circumstances from a new performance feature" is exactly the kind of sentence that makes me want to open the repository. So I did.

It is a good bug. It comes from one of the oldest quirks in PHP, it was triggered by an extension rather than by core, and the fix the team chose is the part I actually want to talk about.

Finding it

Between the 11.0.0-rc.1 tag on July 27 and 11.0.0-rc.2 on July 29, three commits landed on release/11.0. One is an email editor fix. One is a checkout fatal, which I will come back to. The third is titled [Performance] Temporary deactivate product status counters (#67094, cherry-picked from #67058).

"Performance feature", "temporarily deactivate", landed in the RC1 to RC2 window. That is our bug.

The feature

WooCommerce 11.0 was supposed to ship persistent product status counters (#65957), and it is a genuinely good piece of work. It closes issue #31029, open since October 2021, about wp_count_posts() dragging down the WordPress admin.

If you have ever loaded the Products screen on a large catalogue, you have paid for this query:

SELECT post_status, COUNT(*) AS num_posts FROM wp_posts WHERE post_type = 'product' GROUP BY post_status

It is the query behind the little "All (24,318) | Published (22,004) | Draft (12)" links at the top of the list table. It degrades linearly. The benchmarks in the PR: at 50,000 products it costs about a second, at 200,000 products it costs four seconds. Every admin page load. The replacement keeps per-status counts in the object cache, incrementing and decrementing them on woocommerce_new_product, transition_post_status and before_delete_post, with a background Action Scheduler job to warm the cache when it is cold. Four seconds becomes tens of milliseconds.

The fatal

Here is what testers hit:

Uncaught TypeError: Automattic\WooCommerce\Caches\ProductCountCache::get_cache_key():
Argument #2 ($product_status) must be of type string, int given

And the stack trace, which is the interesting part:

#0 src/Caches/ProductCountCache.php(131): ProductCountCache->get_cache_key('product', 0)
#1 src/Internal/Utilities/ProductUtil.php(151): ProductCountCache->set_multiple('product', Array)
#2 includes/class-wc-tracker.php(493): ProductUtil->get_counts_for_type('product')
#3 includes/tracks/class-wc-tracks.php(24): WC_Tracker::get_product_counts()
...
#9 wp-admin/admin-footer.php(78): do_action('admin_footer', '')

Read it from the bottom. This does not fire on the products list. It fires from the usage tracker, on admin_footer, which means on any admin screen. White screen in wp-admin, for a store owner who never went near the Products page.

Now the cause. get_counts_for_type() built its array like this:

$count_per_status = array_merge(
	array_fill_keys( array_keys( get_post_stati() ), 0 ),
	(array) wp_count_posts( $post_type )
);

Then handed it to set_multiple(), which loops over it and passes each key into a method signed get_cache_key( string $product_type, string $product_status ), in a file that opens with declare( strict_types=1 ).

The keys of that array are post status slugs. Strings. Except for one case, and it is a twenty-year-old PHP behaviour that most of us learned once and then filed away: an array key that is a decimal integer string is silently converted to an integer. $a['0'] and $a[0] are the same slot. So a post status named '0' does not stay a string, it becomes the integer 0, and strict_types correctly refuses to coerce it back on the way into a string parameter.

The result is that two safety features doing their job, a typed parameter and strict types, turn a weird status slug into a fatal error instead of a garbled cache key. I do not think that is the wrong trade. It is just an uncomfortable one to discover in an RC.

Where the zero comes from

This is the part I find most familiar. Core does not register a post status called '0'. The reproduction steps in the PR are an mu-plugin:

add_filter( 'wp_count_posts', function ( $counts, $type ) {
	if ( 'product' === $type ) {
		$counts->{0} = 42;
	}
	return $counts;
}, 10, 2 );

That is not a hypothetical. wp_count_posts is a public filter, WooCommerce sits under tens of thousands of extensions, and somewhere out there something is writing a numeric key into that object. The PR is explicit that the source has not been identified yet: the plan is to find "where integer-status is coming from", add normalization, and only then re-enable.

I wrote last week about a Milo bug that only existed on someone else's install, on a roots.io stack where WordPress lives in /wp/. Same shape, much bigger scale. The code was correct against every input the team could imagine. The input came from outside.

The fix is the interesting decision

Here is what I would have expected: add a (string) cast in set_multiple(), or run array_map over the keys, ship the patch, keep the release date. It is a one-line fix and it makes the crash go away.

They did not do that. They turned the entire feature off:

  • get_counts_for_type() goes back to plain wp_count_posts().
  • The load-edit.php priming callback is unhooked.
  • The woocommerce_new_product, transition_post_status and before_delete_post listeners are unhooked.
  • prime_cache_if_cold() becomes a no-op.
  • Four unit tests are marked skipped, with a comment saying why.

And one detail I want to point at, because it is the kind of thing that separates a rollback from a real rollback:

- add_action( 'action_scheduler_ensure_recurring_actions', array( $this, 'schedule_background_actions' ) );
+ add_action( 'action_scheduler_ensure_recurring_actions', array( $this, 'unschedule_background_actions' ) );

They did not just stop scheduling the background job. They actively unschedule it, because sites that already ran RC1 have one sitting in Action Scheduler, and a disabled feature that leaves a recurring job behind is not disabled.

Every line of that change set is commented with the same phrase: Until persistent counters reactivated. Turning something off is easy. Turning it off in a way that the person who turns it back on in three weeks can trust is not.

The reason this is the right call is in the previous section: they do not know where the integer comes from. A cast would silence the symptom and would also silence the report that eventually tells them which extension is writing a 0 key, and whether it is writing anything else. Known-off beats mysteriously-fine.

It was not the only one

Two more fatals landed in the same 48 hours, and they are worth a look because they are three completely different archetypes.

#67100, checkout validation. WC_Checkout::validate_posted_data() built a method name out of a fieldset key:

WC()->customer->{"get_{$fieldset_key}_country"}()

WC_Customer has get_billing_country() and get_shipping_country(). It has nothing for account, order, or any fieldset a plugin registers. So a phone field moved into the order fieldset, which checkout field editor plugins offer as a checkbox, throws. And because process_checkout() catches Exception rather than Error, the request died with a 500 and no order. On checkout.

#67136, the upgrade path. Two REST controllers were deleted in 11.0 along with the old product block editor. But 10.9.4 still registers them unconditionally, and during an in-place update WordPress swaps the files while the old code is still resident in memory. admin-footer.php triggers the wc-admin REST preload, rest_api_init fires, and the bundled Jetpack autoloader hard-requires a file that no longer exists. The update screen dies. The fix is two no-op compatibility stubs, merged to trunk; the backport to release/11.0 was still open when I wrote this.

Dynamic method names, deleted files, and integer array keys. Nothing exotic, no clever code anywhere. All three live at a boundary with somebody else's code.

What I take from it

I have been involved in WooCommerce and WooCommerce.com releases from the inside for eleven years and I now ship a much smaller product with a much smaller team, so let me say the obvious thing plainly: this is a release process working, not failing. The bug was found in RC1, by testers, before a single store took it. The release moved by seven days. Nobody shipped a hardening patch they did not fully understand in order to protect a date.

The temptation to hold the date is enormous, and I know it personally. A week of slip is visible to everyone. A fatal in wp-admin is visible to everyone too, except it arrives with your name on it and a support queue attached.

The other lesson is the one I keep relearning at every scale. The code in all three of these was fine. What broke it was the surface it exposes: a public filter, a dynamic method name, a class that other people's code still instantiates. On a platform, the dangerous code is not the complicated code. It is the code with a door in it.

11.0 is due August 4. The counters will come back once someone finds out who is writing a zero. Props to the WooCommerce team for their hard work and dedication.