Jetpack: Bug: PHP Warning - plugin on your site is using the plugins_loaded filter with a priority that is too high

[amElnagdy]

Steps to reproduce the issue

1. Install WPML
2. Install Jetpack
3. Enable WP_DEBUG
4. Get the PHP Warning
   Notice: A plugin on your site is using the plugins_loaded filter with a priority that is too high. Jetpack does not support this, you may experience problems. in /public_html/wp-content/plugins/jetpack/class.jetpack.php on line 573

What I expected

If you want to inform a user about a problem, you should display an admin notice on the backend, not by throwing a PHP notice.

WPML is using
add_action( 'plugins_loaded', 'otgs_ui_plugins_loaded', -PHP_INT_MAX );

[jeherve]

Related: #14720

If you want to inform a user about a problem, you should display an admin notice on the backend, not by throwing a PHP notice.

Those notices are mostly meant for plugin developers, who will be in a position to update their plugin accordingly.

@vukvukovich Hi! 👋 Do you think you could take a look at this, since you work on the WPML team?

Thank you!

[amElnagdy]

@jeherve Thanks for checking this out.
I understand this was meant for devs, but it's throwing an error on every site that has WP_DEBUG enabled.
WPML is using -PHP_INT_MAX priority and I don't think it's forbidden. What do you think?

PS: I work on the WPML team too.

[jeherve]

I work on the WPML team too.

Ah, perfect then, it reached the right person then!

it's throwing an error on every site that has WP_DEBUG enabled.

Right, this was the idea. It's a good way to reach out to developers without impacting production sites.

WPML is using -PHP_INT_MAX priority and I don't think it's forbidden. What do you think?

It certainly is not forbidden, but it does make things more complicated for other plugins in the ecosystem. May I ask why you took this approach with WPML in the first place?

[kraftbj]

For the sake of sharing, we added this because of a conflict with a couple different plugins, including WPML.

https://wpml.org/forums/topic/woocommerce-mobile-app-wpml-plugin-conflict/

We don't have a copy of WPML, but the issue was easily reproducible with the snippet seen in #14720.

An error was occurring for REST API interactions that were being proxied through Jetpack from the public-api.wordpress.com API. With the Jetpack connection, app developers can create an app with WordPress.com and then site owners can opt to authorize that app to either a WordPress.com site or a Jetpack site.

In our case, we were initially hooking our code on plugins_loaded priority 1 to add the authorization bits, but an issue came up when a site is calling wp_get_current_user() among other functions in code hooked onto plugins_loaded earlier, we would have issues.

We need to call it at plugins_loaded because the code is within Composer packages that are intended to be used on multiple plugins and we have an autoloaded that determines which version to load. We can't do that until all plugins have been loaded so we know what versions are requested by the various plugins.

[strategio]

Hi @jeherve, I am the compatibility team leader at WPML.

We have several components that we need to load as soon as possible. We do this for different reasons, but I can share some:

• We have a sub package shared by different plugins from our company (not only WPML) and we want to select the most recent version before to use it.
• We need to track the MO files loaded by all plugins.

Using -PHP_INT_MAX does not guarantee we will always be the first to load, but it helps. In some cases, we also have some extra logic to recover the information that we could have lost if a plugin was loaded before WPML.

What I would suggest you is to display a (dismissable) admin notice explaining what you detected and what could be the possible consequences (just an informative message).

On our side, we have quite a lot of hooks loaded with the priority -PHP_INT_MAX and we have no plan to change it for now. So triggering this error will have no effect for us as plugin developers but it will just cause troubles to our (and your) users.

[kraftbj]

@strategio Looks like we're trying to solve the same problem here and it'd be great if we could figure out a better solution together than -PHP_INT_MAX and hoping it works out. The goal with the debug notice was to help start conversations when other plugins are also trying to load things extremely early on that hook and it worked. I'd wager we aren't going to be the only ones trying to do this with shared components.

With the admin notice, I don't think that's a great solution since we're now offloading the technical difficulty onto the users who would be less able to solve it than we are. Perhaps, a card in the Site Health section, but I'd rather us figure out a user-centric solution other than basically, possibly, asking folks to pick between us.

In the name of compatibility, let's see if we can figure out a different solution that we may be able to share with others in the ecosystem.

[strategio]

@kraftbj, I don't understand why you first try to configure your action on plugins_loaded with priority 90:

add_action( 'plugin_loaded', array( $this, 'add_configure_hook' ), 90 );

Why not immediately $this->add_configure_hook(); in the constructor? Is there a specific reason for using 90?

That way, you could also try to reorder the actions if you detect that there's another one with the same -PHP_INT_MAX priority.

In your current code it might be a bit late to change the priorities.

[kraftbj]

That's on plugin_loaded, not plugins_loaded. We wanted to run it later than default for after an individual plugin has been loaded so we could check for if other things have been loaded on plugins_loaded.

Our idea was we wanted to try to load first, but still giving a way (e.g. through a theme's functions.php) to load earlier for a site-specific reason.

By using plugin_loaded, we're checking after each plugin (post-Jetpack) is loaded and hooking our configure action at one level earlier than what that plugin is doing instead of going to -PHP_INT_MAX right out of the gate.

So, in our initial case, we were loading at priority 1 and got bit when a plugin was adding themselves to priority 0 on the plugins_loaded hook. In that case, we would load ourselves at -1.

[strategio]

Ah my bad, I missed the s in the hook name :)

I could do this yet, but I'll try to think of another approach when I have some time.

[kraftbj]

Sounds good. Definitely open to different approaches and I don't know the right solution yet either. The downside to our approach is if we both do it, we'll run into the same issue where eventually one of us will be before the other and still hoping to chance.

Jetpack will probably need to have some better logic in cases like we had when WPML ran before Jetpack on plugins_loaded to prevent severe issues, but doesn't solve the underlying potential conflict.

[strategio]

@kraftbj, @jeherve, here's a suggestion https://github.com/Automattic/jetpack/pull/14771.

The first time we call add_configure_hook, we add the callback. And then we will always make sure that the jetpack action if the first one in the top priority.

I don't think it will cause problems on WPML side if you rob the first top priority, but we can still use - PHP_INT_MAX which as I said we are not planning to change.

Let me know your thoughts.

[kraftbj]

With the discussion and logic change being developed in 14771, we feel confident we can have something that meets our goals without the PHP_INT_MAX concerns.

The changes are a bit bigger though than we'd like this late in the cycle (post-beta, release on Tuesday). Since we have a solid plan forward that resolves the concerns, we are going to remove the notice in 8.3 and land the new solution in 8.4.

Thanks all!

[stale[bot]]

This issue has been marked as stale. This happened because:

• It has been inactive in the past 6 months.
• It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.