humanmade / Cavalcade

A better wp-cron. Horizontally scalable, works perfectly with multisite.
https://engineering.hmn.md/projects/cavalcade/
Other
519 stars 46 forks source link

Cron filters are not registered early enough #101

Closed dd32 closed 4 years ago

dd32 commented 4 years ago

Currently the various Cron Connector filters are added on plugins_loaded at the default priority of 10, unfortunately this means that it's possible for other plugins hooks to be run at earlier priorities on the hook or even just because they were registered first.

This can have the effect the wp_next_scheduled() and/or _get_cron_array() calls return incorrectly.

This is made worse by Jetpack which forces itself to be the first plugins_loaded callback which ultimately causes cron functions to be called.

this can be seen by something like this:

add_filter( 'plugins_loaded', function() {
    var_dump( get_option( 'cron' ) );
        die();
 }, 0 );

looking at the output, you'll see that the cron events are coming from the WordPress cron option, rather than from Cavalcade. Bump the priority to 100 and you'll see that it's Cavalcade cron tasks.

rmccue commented 4 years ago

IMO, if you're performing side-effects on plugins_loaded, you'll almost always run into issues like this. That said, we can lower the priority a bit to account for the common mistakes.

I don't think we can account for the Jetpack case though; that seems like Jetpack doing things wrong.

dd32 commented 4 years ago

IMO, if you're performing side-effects on plugins_loaded, you'll almost always run into issues like this.

I agree.

That being said, there's no good reason for Cavalcade delaying registering it's actions until the plugins_loaded hook IMHO.

roborourke commented 4 years ago

I think this is a duplicate of #83 ultimately - lets continue the discussion there.