humanmade / Cavalcade

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

doesn't support __invoke()? #54

Closed lougreenwood closed 6 years ago

lougreenwood commented 6 years ago

It seems that perhaps the Cavalcade WP plugin doesn't support adding a job using a class with an invokable as the callback?

http://php.net/manual/en/language.oop5.magic.php#object.invoke

I have the following code to add my invokable class:

if ( ! wp_next_scheduled( Model\MyExampleInvokableClass::class ) ) {
    wp_schedule_event( time(), 'daily', Model\MyExampleInvokableClass::class );
}

This seems to work fine using the standard WP Cron system and I'm able to verify that the job is queued using the 'Advanced Cron Manager' plugin (https://wordpress.org/plugins/advanced-cron-manager/).

However, when Cavalcade is enabled, these jobs disappear from the jobs list in WP Admin and also aren't visible when I do wp cavalcade jobs.

I don't see anything in PHP error logs or cavalcade log either...

Thanks! 😃

rmccue commented 6 years ago

Keep in mind that what you're scheduling is the hook name, not the callback. There's no restrictions on the hook name in WP that are relevant here, so my guess would be that you're not hooking into this correctly. The relevant code for WP is in connector/namespace.php, which is called via wp_schedule_event's update_option call. It's possible that wp_schedule_event is returning false for some reason (invalid timestamp, invalid recurrence, or something hooked into schedule_event)

I'd check the database table (wp_cavalcade_jobs) directly and see if your job has been scheduled there.

Happy to look further if you want to share your full code for the event hooking, but I don't think this is a core Cavalcade issue. :)

lougreenwood commented 6 years ago

Sorry for the delayed reply, been super busy with a deadline.

I've just done some more investigation and it seems that I'm not able to register any cron jobs with Cavalcade enabled. The only jobs that cavalcade jobs reports are:

+----+------+---------------------------+---------------------+---------------------+---------+
| id | site | hook                      | start               | nextrun             | status  |
+----+------+---------------------------+---------------------+---------------------+---------+
| 15 | 1    | wp_version_check          | 2018-03-04 16:17:12 | 2018-03-05 04:17:12 | waiting |
| 16 | 1    | wp_update_plugins         | 2018-03-04 16:17:12 | 2018-03-05 04:17:12 | waiting |
| 17 | 1    | wp_update_themes          | 2018-03-04 16:17:12 | 2018-03-05 04:17:12 | waiting |
| 18 | 1    | pj_transient_cleaner      | 2018-03-04 16:17:12 | 2018-03-05 16:17:12 | waiting |
| 19 | 1    | wp_scheduled_delete       | 2018-03-04 16:17:12 | 2018-03-05 16:17:12 | waiting |
| 20 | 1    | delete_expired_transients | 2018-03-04 16:17:12 | 2018-03-05 16:17:12 | waiting |
+----+------+---------------------------+---------------------+---------------------+---------+

cavalcade log has no info about the missing cron jobs, neither do the cavalcade db tables.

I have a suspicion that this is a problem related to my setup. I'm using the "Themosis" framework. It seems that when Cavalcade is enabled, the Cavalcade schedule_event() function isn't called. However, I added some var_dumps to wp-includes/cron.php and I see that my expected jobs are at least received by WP.

So I suppose that Cavalcade isn't properly hooking into when WP is setup as a Themosis project.

I think my code will be too complex to share here, but I'm happy to grant you access to our repo and help you spin up a docker container for our project - assuming you're happy to help debug further?

Edit: Themosis framework: https://framework.themosis.com

rmccue commented 6 years ago

Cavalcade hooks in at a deep level in WordPress, so themes shouldn't be able to affect it. I think it's more likely that Cavalcade isn't actually being loaded properly. Are you sure the Cavalcade plugin is enabled?

I can try and find time to look at the repo if you want to give me access, but I can't guarantee any timeframe for doing so. You're best off digging into debugging it further yourself if you can.

lougreenwood commented 6 years ago

Hi Ryan,

Is it possible to describe how Cavalcade is hooking in, I can take another look.

Regarding Themosis, it’s a framework which make WordPress more like a traditional MVC framework. We’re using it to make WP more usable as a headless-only REST API server for a complex web app. It modifies the bootstrap process for WP and makes some use of Laravel to add some extra features (routing, container etc) and allows managing WP with composer.

So more complex than just another theme, which is where I think that the conflict is coming from.

On 5 Mar 2018, at 02:06, Ryan McCue notifications@github.com wrote:

Cavalcade hooks in at a deep level in WordPress, so themes shouldn't be able to affect it. I think it's more likely that Cavalcade isn't actually being loaded properly. Are you sure the Cavalcade plugin is enabled?

I can try and find time to look at the repo if you want to give me access, but I can't guarantee any timeframe for doing so. You're best off digging into debugging it further yourself if you can.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/humanmade/Cavalcade/issues/54#issuecomment-370289768, or mute the thread https://github.com/notifications/unsubscribe-auth/ALflGHbMs_69nnjIkGF-qafUT_DW4rJgks5tbJ2sgaJpZM4Rv2zX.

lougreenwood commented 6 years ago

Another thing to mention. When I do wp_get_schedule() or wp_next_scheduled(), the jobs which I'm registering, but Cavalcade is missing are returned by WP, so they're getting registered, it seems they just aren't getting 'synced' to Cavalcade.

It's also curious that the pj_transient_cleaner job (in my earlier post) which Cavalcade does pick up isn't a core WP job, (the rest of the jobs are WP native ones) - so maybe there's a hint for me in there too...

And yep, Cavalcade plugin is enabled in mu-plugins. I've debugged in there to to verify it. Bootstrap creates/populates the tables etc

rmccue commented 6 years ago

Specifically, Cavalcade hooks in at pre_option_cron and pre_update_option_cron, which are the low-level option hooks in WordPress. This means that any higher level functions (including wp_get_schedule() etc) will use this data automatically. You can check out how this works in the connector namespace.

A tell-tale sign that Cavalcade is enabled is that entries in the cron array will have 'schedule' => '__fake_schedule'. (This is because Cavalcade only stores the interval, not the full schedule ID.) If you're spotting those in the data, then Cavalcade is actually enabled. If not, then Cavalcade isn't enabled.

My guess here is that maybe jobs are being scheduled before Cavalcade is actually loaded. Unless Themosis also has hooks in pre_option_cron etc then it shouldn't be possible for it to be affecting Cavalcade directly, and it's very unlikely that it would be doing that. The most likely explanation is that Cavalcade isn't loaded when your jobs are being scheduled.

I'd also advise double-checking the database directly (specifically the cavalcade_jobs table) just to make sure the wp-cli command is actually returning the right thing.

lougreenwood commented 6 years ago

@rmccue Good news, I was able to get the jobs loading by registering them using the 'init' action hook. The jobs are now properly scheduled.

Thanks for your help, much appreciated!

Cheers

rmccue commented 6 years ago

Fantastic news, thanks for the update!