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

Defensive checks in cron rescheduling #53

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

GiuseppeArcifa
Copy link

Proposed changes

This PR includes a code refactoring of the TaskManager section and additional checks to unsure that the Task class is being instanced if its managed the cron is scheduled.

In addition the custom schedules are included from a new TaskManagerSchedules class that handles them.

There was a lot of repetitive code that was abstracted and put in an AbstractTaskManager class that then is extended by all the other tasks manager classes.


Initially this PR was meant to solve issues related to cron that were randomly displaying in the log, such as:
Cron unschedule event error for hook: nfd_module_installer_plugin_deactivation_event, Error code: could_not_set, Error message: The cron event list could not be saved., Data: {"schedule":false,"args":[]}

This kind of issue present when a cron is executed multiple and simultaneous times, so that trigger an error with the cron option update. This is an issue that affect WordPress since 6.1 version. (in the last section i'm attaching the related WP Core thread)

For this PR we are including the refactoring and additional checks and cron clearing after their completion as is the best we can do for the moment. We can evaluate further investigation for the future.

Type of Change

Production

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update
  • Refactoring / housekeeping (changes to files not directly related to functionality)

Development

  • Tests
  • Dependency update
  • Environment update / refactoring
  • Documentation Update

Video

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Here the thread related to the WordPress Cron issue:

…l Task Manager classes, reducing redundancy and improving code clarity and maintainability.

Modified Task Managers to instantiate only when their associated WP Cron hooks are scheduled, optimizing resource usage and avoiding errors about unhandled hooks.
Ensured recurring WP Cron hooks are cleared after completion using `wp_clear_scheduled_hook`, preventing unnecessary executions.
Added `TaskManagerSchedules` class to register new custom schedules for WP Cron jobs via filters, enabling more flexible scheduling for various Task Managers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant