[WOOTAX-299] Fix - Don't register the continents fallback on unrelated REST requests

[bartech]

Description

WC_Connect_Loader::wc_api_dev_init() decides whether WooCommerce provides /wc/v3/data/continents — and if it thinks not, registers the bundled classes/wc-api-dev/ copy as a fallback. It made that decision by looking for the route in the current request's route table:

$existing_routes = rest_get_server()->get_routes();
if ( ! isset( $existing_routes['/wc/v3/data/continents'] ) ) { … }

That inference was correct when it was written and stopped being correct in WooCommerce 9.2. Core now registers the wc/v3 namespace lazily — Server::get_rest_namespaces() consults wc_rest_should_load_namespace(), which returns false whenever the current request targets a different known namespace (wc/store, wc-analytics, wc-admin, wc/v1, wc/v2, wc-telemetry, wc/private). On every one of those requests /wc/v3/data/continents is legitimately absent even though core supplies the controller, so the guard passes and the fallback fires: two require_onces, a controller construction, and two register_rest_route() calls, on every block-cart and Analytics request for every shipping-enabled install.

Nothing observable comes of that, and the PR should not be read as a performance fix. Route registrations last exactly one request, and a request aimed at wc/store can never dispatch to /wc/v3/data/continents — so no wrong answer is ever served, and on requests that do target wc/v3 core registers at priority 10, our guard sees the route, and we skip as we always have. What the fallback does on those other requests is dead work on a hot path: real and measurable, but microseconds.

The case for fixing it is that the guard's logic is now wrong, and incorrect logic on a hot path is what let an already-fixed crash survive in production for two years — see below.

The fix is to ask whether core has the controller, not whether the route happens to be registered for this request. class_exists( 'WC_REST_Data_Continents_Controller' ) resolves through WooCommerce's autoloader, whose wc_rest_ branch searches includes/rest-api/Controllers/Version{1,2,3}/ — so it is definitive regardless of what has been loaded so far. Core has shipped this endpoint since WC 3.5 (2018), well under this repo's WC 9 floor, so the fallback now stays dormant everywhere it should and still engages if the endpoint is ever genuinely missing. The old route-table check is kept underneath as that genuine "core is too old" path.

This is also the branch behind the WP Cloud "Cannot call constructor" fatals

WOOTAX-299 reports ~1,413 Uncaught Error: Cannot call constructor fatals across 56 WP Cloud sites, thrown from class-wc-rest-dev-data-continents-controller.php:47, and hypothesises that a WooCommerce core base-controller constructor changed under us. That diagnosis does not hold, and this PR is not the fix for it — but it is the fix for what kept detonating it.

• The crash itself was fixed in 2.5.1 (Feb 2024), by 1657ca7, which deleted a parent::__construct() call. Current stable is 3.6.11.
• The Grafana query's line:47 facet is a version fingerprint. In 2.5.0 line 47 was parent::__construct();. Post-fix, line 47 is $this->continents = new WC_Connect_Continents();. PHP emits Cannot call constructor only from the explicit Parent::__construct() call form against a constructor-less class — new Foo() on such a class is perfectly legal. So the reported rows can only come from installs still on ≤ 2.5.0.
• It was never a version-compat regression. No class in the ancestry — WC_REST_Dev_Data_Controller → WC_REST_Controller → WP_REST_Controller — has ever declared a constructor. That call was unconditionally fatal from the day it was written, on every WooCommerce and every PHP 7/8. It only looked version-dependent because the branch reaching it is.

So a two-year-old, already-fixed bug still fires fleet-wide because WC 9.2 turned its trigger from "essentially never" into "most REST requests".

This PR does not help the 56 sites in that report — they crash because they run a 2.5-year-old plugin, and the only cure is updating it. The one claim in the diagnosis not yet verified from primary data is that those rows really do carry plugin version ≤ 2.5.0; the Grafana query has no version facet today. That question is tracked separately so it outlives this PR — see Related issue(s).

Second, smaller fix: the fallback did not require its own dependency

WC_REST_Dev_Data_Continents_Controller::__construct() builds a WC_Connect_Continents, but the branch only required the two controller files and relied on load_dependencies() having required class-wc-connect-continents.php earlier. In production it always has, so this is latent rather than live — but the first test run surfaced it as a concrete Error: Class "WC_Connect_Continents" not found at that same line 47. The fallback now requires it itself and is self-contained.

Since review, that branch is covered by test_wc_api_dev_init_registers_bundled_continents_route_when_core_lacks_it, which stubs the new core_provides_continents_controller() seam to false and runs the real wc_api_dev_init(), asserting both bundled routes register. Dropping any of the branch's three require_once calls fails that test when the file runs on its own. The WC_Connect_Continents one is masked in a whole-suite run, because an earlier test file has already loaded that class for its own setup — loading a file is process-global and monotonic, so the only way to observe it is a fresh process per test, which this suite cannot afford (PHPUnit's process-isolation annotation costs ~26s for one test and its child bootstrap reinstalls the shared test database underneath the tests that follow). That limit is documented on the test.

Backward compatibility

Public and externally exposed surface touched. No signature changes.

• WC_Connect_Loader::wc_api_dev_init() keeps its signature, its visibility, and its rest_api_init priority-9999 hook. It is still registered under the same should_load_shipping_features() gate.
• WC_REST_Dev_Data_Continents_Controller and WC_REST_Dev_Data_Controller are untouched — not renamed, not deprecated, not removed. Both are global-namespace classes that out-of-repo code could subclass or instantiate, and both still load and behave identically when the fallback engages.
• No hook is added, removed, reordered or retimed. No new filter or action, and no change to when any existing one fires.
• The only behaviour change is that the plugin no longer registers a route on requests that could never have dispatched to it. A route registered during a wc/store/v1/cart request only exists for the lifetime of that request, and that request cannot route to /wc/v3/data/continents. There is no way for a consumer to observe the difference. On a request that does target wc/v3, wc_rest_should_load_namespace() returns true, core registers its own controller, and the endpoint responds exactly as before — from core's implementation, which it already did.
• No new global state is read. class_exists() and require_once only; no WC()->… dereference, no $post, no $wp_query, no session or cart access. Safe on the REST, cron, WP-CLI and webhook paths.
• Multisite: unchanged. No option is read or written, site-scoped or network-scoped.
• Install layout: unchanged. Paths are still built from __DIR__; no URL is constructed.
• Shipping compatibility: this code path is reached only when should_load_shipping_features() is true, i.e. on grandfathered installs. Nothing is removed from that surface — the fallback still exists and still works.
• One method added: protected WC_Connect_Loader::core_provides_continents_controller(). Additive and protected, so no existing signature or visibility changes; the only way it can break a consumer is a subclass that already declares a member of that exact name with incompatible visibility. Behaviour is identical — it returns the same class_exists( 'WC_REST_Data_Continents_Controller' ) the guard evaluated inline before, and it exists so the fallback branch is reachable from a test.

Related issue(s)

WOOTAX-299 — auto-linked via the branch name; this PR carries the code half of it.

WOOTAX-323 — the remaining half: confirm from Grafana that the affected rows carry plugin version ≤ 2.5.0. Fleet work rather than code work, and it is what decides whether ~1,413 fatals/week are already remediated or whether there is a live crash we have not found.

Same alert family, and both are worth re-checking with the line-number-as-version-fingerprint technique above before any code is written for them:

• WOOTAX-298 — Class Automattic\WooCommerce\StoreApi\StoreApi not found
• WOOTAX-297 — Failed opening required 'jetpackclass-wc-connect-api-client.php'

Follow-up worth filing: retire the classes/wc-api-dev/ shim entirely. Its own docblock says "Delete this when the 'v3' REST API is included in all the WC versions we support", and it is. Per AGENTS.md that has to be a deprecation rather than a deletion, since both classes and the loader method are public globals.

Steps to reproduce & screenshots/GIFs

Automated — this is the primary evidence.

1. composer test → 404 tests / 985 assertions green, zero regressions (403 / 982 before the review-round test was added).
2. Fail-first confirmed. Stash only woocommerce-services.php and re-run
   composer test -- --filter test_wc_api_dev_init_defers_to_core_continents_controller.
   It fails — and fails at the exact crash site named in the ticket:

   Error: Class "WC_Connect_Continents" not found
     .../classes/wc-api-dev/class-wc-rest-dev-data-continents-controller.php:47
     .../woocommerce-services.php:1306
   

   That is the defect reproduced from primary evidence: against WooCommerce 10.9.1, the fallback branch really is entered on a Store API request. (It surfaces as class-not-found rather than "Cannot call constructor" because the test builds the loader via mockLoader(), which skips load_dependencies() — which is what prompted the second fix above.)
3. PHPCS: zero new violations. phpcs.xml.dist --report=source over both changed files reports 206 SNIFF VIOLATIONS … IN 29 SOURCES before and after, with identical per-sniff counts.

Manual, on a store with shipping features enabled (should_load_shipping_features() true), WooCommerce ≥ 9.2:

1. Add error_log( 'wc-api-dev fallback fired' ); inside the ! isset( $existing_routes[…] ) branch of wc_api_dev_init().
2. Load the block cart, or open WooCommerce → Analytics (either issues wc/store / wc-analytics REST requests).
3. Before: the line appears in debug.log on essentially every such request. After: it never appears.
4. GET /wp-json/wc/v3/data/continents as an admin still returns the full continents payload in both cases — served by core's controller, as it already was.

Checklist

• unit tests

• changelog.txt entry added
• readme.txt entry added

[CezaryDrewniak]

Reviewed the follow-up and the fix holds up.

I ran the reset mutation ($non_taxable = array( $canonical_id => true )) against the updated seam test: it fails with the first exempt key missing from the map — precisely the WOOTAX-240 shape this test exists to catch. Suite green at 401/978, PHPCS clean (no new violations on either file), and the class file is byte-identical across the two commits, so no production risk. Also traced every consumer of the map and canonical ids — nothing else is affected.

[Abdalsalaam]

woocommerce-services.php:1321 adds require_once __DIR__ . '/classes/class-wc-connect-continents.php'; to the fallback branch, presented in the PR body as the fix for the observed Class "WC_Connect_Continents" not found error. But test_bundled_continents_controller_is_constructible requires that same file itself before constructing the controller, and test_wc_api_dev_init_defers_to_core_continents_controller returns at the new class_exists() guard and never enters the fallback branch. Deleting line 1321 from production leaves the entire suite green, so the test proves the controller is constructible GIVEN its dependency rather than that the fallback loads it.

Suggested change

	require_once __DIR__ . '/../../classes/class-wc-connect-continents.php';
	require_once __DIR__ . '/../../classes/wc-api-dev/class-wc-rest-dev-data-controller.php';
	require_once __DIR__ . '/../../classes/wc-api-dev/class-wc-rest-dev-data-continents-controller.php';
	require_once __DIR__ . '/../../classes/wc-api-dev/class-wc-rest-dev-data-controller.php';
	require_once __DIR__ . '/../../classes/wc-api-dev/class-wc-rest-dev-data-continents-controller.php';

[bartech]

You're right that nothing pinned that require. I checked it the way you did and got the same result: delete line 1321 and the whole suite stays green, because neither test ever enters the fallback branch. Fixed in 8a1691a.

I did not apply the suggested diff, though, because I don't think it gets there. I tried it:

• on its own, composer test -- --filter test_bundled_continents_controller_is_constructible fails with Class "WC_Connect_Continents" not found
• the full composer test passes

It passes in the suite only because tests/php/classes/test-class-wc-rest-connect-shipping-label-controller.php sorts earlier and loads that class for its own setup. So the test would be relying on process-global class-load state again — the thing your first comment (correctly) got rid of — and it still never enters the fallback branch, so line 1321 would stay unpinned either way.

What I did instead was make the branch reachable. wc_api_dev_init()'s "does core supply the controller?" question now lives in a small protected method, so a test can answer it "no" on a WooCommerce that does supply it. Everything else in the method runs for real: the three require_once calls, the controller construction, and register_routes(). The new test asserts both bundled routes show up on a route table where core's wc/v3 namespace is absent, so they can only have come from our fallback.

Mutation results, all with --filter test_wc_api_dev_init:

Mutation	Result
drop $continents->register_routes();	fails
drop either wc-api-dev require	fails, class not found
drop the WC_Connect_Continents require	fails, class not found
guard back to route-table-only (the bug this PR fixes)	fails, on the sibling test

One honest limit: that third row only holds when the file runs alone. In a whole-suite run an earlier file has already loaded WC_Connect_Continents, so the missing require is invisible. Loading a file is process-global and monotonic, so the only way around it is a fresh process per test, and that is not affordable here — I measured PHPUnit's process-isolation annotation at ~26s for a single test (the child re-runs the WP/WC bootstrap), and that bootstrap reinstalls the shared test database underneath the tests that run after it. I wrote that limit into the test's docblock rather than leave it implied.

Suite is 404 tests / 985 assertions green, and PHPCS is unchanged on both files (206 violations / 29 sources before and after, same path).

[CezaryDrewniak]

Reviewed this end to end: code, tests, and against the WC/WP internals. Looks good.

Verdict

Approve with one wording nit in the BC section (see below).

The fix is right: class_exists( 'WC_REST_Data_Continents_Controller' ) resolves through WooCommerce's autoloader (checked 9.4 and 10.4 — the file only exists under Version3/, no name collisions), so it's a definitive "does core provide the endpoint" check regardless of what the current request has loaded. The old route-table guard genuinely broke on WC 9.2+ lazy namespaces (wc_rest_should_load_namespace()), and the new guard keeps the fallback dormant everywhere it should while the route-table path stays as the genuine "core is too old" branch.

Tests — all verified

• Full suite: OK (404 tests, 985 assertions).
• All three new tests pass in isolation.
• Fail-first reproduced: with trunk's woocommerce-services.php temporarily restored, test_wc_api_dev_init_defers_to_core_continents_controller fails at line 137 (/wc/v3/data/continents present in the route table on a wc/store request) — and it's the only failure in the whole suite (404 tests / 985 assertions), so nothing else regresses.
• Reran the two guard tests on WooCommerce 9.4.0: green (OK (2 tests, 6 assertions)).

PHPCS

No new violations on either changed file — zero hits in woocommerce-services.php:1300-1345 or in the new test methods. (The repo-wide run still reports the pre-existing backlog, unrelated to this PR.)

One BC note

The "no way for a consumer to observe the difference" sentence in the BC section is slightly stronger than reality. A site that filters wc_rest_should_load_namespace to force wc/v3 off even on v3 requests would previously have had the bundled fallback register and serve the route; with this change the endpoint 404s (alongside the rest of that namespace). That's a deliberate and sensible trade — the site already opted out of wc/v3 — but I'd soften that one sentence to acknowledge it rather than claim zero observable difference.