Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preloading the REST API causes issues with manual plugin updates #47266

Closed
5 tasks done
RadoslavGeorgiev opened this issue May 8, 2024 · 2 comments
Closed
5 tasks done
Labels
focus: install-uninstall Issues related to installing, uninstalling, and upgrades. focus: rest api Issues related to WooCommerce REST API. team: Proton

Comments

@RadoslavGeorgiev
Copy link
Contributor

RadoslavGeorgiev commented May 8, 2024

Prerequisites

  • I have carried out troubleshooting steps and I believe I have found a bug.
  • I have searched for similar bugs in both open and closed issues and cannot find a duplicate.

Describe the bug

At Transact, we have faced multiple issues while testing new WooPayments plugin versions, and it seems the reason is that the REST API is being initialized at inappropriate times.

Related to this topic, we some of the discussions we had include paJDYF-aZa-p2, p1715098461138369-slack-CGGCLBN58, and Automattic/woocommerce-payments#7464. However, we have faced similar issues upon multiple releases.

I can see that others have experienced similar behavior in different contexts, including upon Woo updates:

  1. Error on plugin screen when Beta Tester installed and WooCommerce is updated via plugin upload #44373
  2. [7.9] Fatal Error: GenericController not found in API/Reports/Controller.php #39036
  3. Got fatal error after update to wc 3.7 #24386

Initializing the REST API can cause hidden delays on most, if not all, admin pages. On a random Jurassic Ninja site with Woo and WooPayments active, REST API preloading adds about ~120ms to a ~1000ms page load, which is not massive, but not negligible either.

The bigger issue we're facing is upon manual plugin updates. Here's what happens:

  1. Before processing the uploaded file, all generic WP hooks are called, initializing plugins.
  2. The plugin files are replaced, be it WooPayments, Woo, or another extension.
  3. After the replacement, in the admin footer, the REST API is initialized, causing controller files to be loaded, and classes to be instantiated. In this case the newly uploaded constructors are used, but in an outdated environment, triggering an error.

The described behavior happens anytime REST controller constructor parameters are added, or rearranged. The error that appears looks like this:

image

Once the error is displayed, there are no further side effects, as on subsequent page loads, the right environment is in place. However, this causes concerns with both developers and merchants.

Expected behavior

REST API endpoints should only be preloaded in applicable contexts, whenever it is actually needed.

Actual behavior

rest_preload_api_request() is called upon any admin page visit.

Steps to reproduce

I will not add details on how to make the error appear, just how to simulate the environment:

  1. Add breakpoints the following couple of palces
    $preload_data_endpoints = apply_filters( 'woocommerce_component_settings_preload_endpoints', array() );
    $preload_data_endpoints['jetpackStatus'] = '/jetpack/v4/connection';
    if ( ! empty( $preload_data_endpoints ) ) {
    $preload_data = array_reduce(
    array_values( $preload_data_endpoints ),
    'rest_preload_api_request'
    );
    }
    $preload_data_endpoints = apply_filters( 'woocommerce_component_settings_preload_endpoints', array() );
    $preload_data_endpoints['jetpackStatus'] = '/jetpack/v4/connection';
    if ( ! empty( $preload_data_endpoints ) ) {
    $preload_data = array_reduce(
    array_values( $preload_data_endpoints ),
    'rest_preload_api_request'
    );
    }
  2. Upload a plugin, any plugin.
  3. Notice how even in this weird scenario, REST API data is being preloaded for the footer.

The whole process relies on AssetDataRegistry checking whether a specific script, ex. wc-settings is being enqueued. This seems to be a bit inconsistent by itself, so replacing this line with if ( true ) { helps with debugging:

if ( wp_script_is( $this->handle, 'enqueued' ) ) {

WordPress Environment

Nothing specific, this issue has persisted throughout multiple versions.

Isolating the problem

  • I have deactivated other plugins and confirmed this bug occurs when only WooCommerce plugin is active.
  • This bug happens with a default WordPress theme active, or Storefront.
  • I can reproduce this bug consistently using the steps above.
@jonathansadowski jonathansadowski added focus: rest api Issues related to WooCommerce REST API. team: Proton focus: install-uninstall Issues related to installing, uninstalling, and upgrades. labels May 8, 2024
@coreymckrill
Copy link
Collaborator

@RadoslavGeorgiev If I'm understanding this correctly, we actually have an active issue right now that I think might address this: #44539

Do you think this idea of only loading on demand would resolve this issue?

@coreymckrill
Copy link
Collaborator

Going to close this as a duplicate of #44539, but please reopen if there's something here that's not captured in the other one.

@coreymckrill coreymckrill closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: install-uninstall Issues related to installing, uninstalling, and upgrades. focus: rest api Issues related to WooCommerce REST API. team: Proton
Projects
None yet
Development

No branches or pull requests

3 participants