Simple cron log failed executions.

please review the MR @O'Briat , thanks

We push simultaneously :)

Anyway the MR is ready to be RTBC

I mark this issue as RTBTC as it run smoothly on production websites for 4 months

I need more testing, this patch add too much logging, CronJob::run already log successful and failed job

O'Briat → changed the visibility of the branch 3379107-simple-cron-should to hidden.

Additionally, if the Simple Cron job fails, it does not update the last run. Let me explain why this could be a potential issue.

In our project, we have a Simple Cron job that loops through a set of emails and send a notification (based on a condition: If date is 14 days from today).
This cron should execute once a day if all goes well. If the cron job fails, there is no long showing that it failed, and the last run is not updated.
The core-cron is set to run every 20 minutes. On the next core-cron run, it will check for last run time for the simple cron job that failed and will not find it and execute it again until it fails again. If it failed after sending 20 emails, and there are still 30 more to go. The first 20 emails will get repeated emails every time the core-cron runs (20 minutes).

Solution:
The simple cron job, should update last run time even if it fails. The responsibility of making sure it doesn't fail belongs to the developer.

Fine to create as a separate issue I think. This one is okay, but coding standards should be followed (which currently are not). Updated issue summary to reflect that.

Phpcs fixed, I also added two minors fixes on CronTest.php

Thanks! CronTest.php phpcs seems like its out of scope, I don't think phpcs should be applied to existing code as part of this issue, that is getting resolved in other issues.

Yes, I know but with just two carriage returns all the module now complies with the Drupal standards. If you think it's not needed, I'll revert it.

Nah all good, does no harm, ignore me :) Anyways I put in an application to take over maintenance since the maintainer has been quiet for a long time now. If that does go ahead I'll get a bunch of issues merged in + test coverage running again once 📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review is in

scott_euser → changed the visibility of the branch 3379107-simple-cron-should to active.

Thanks!

Solution:
The simple cron job, should update last run time even if it fails. The responsibility of making sure it doesn't fail belongs to the developer.

P.S. If this needs a separate issue, I can create that. It's a simple one, this.

Yes if you could create a separate issue please, as may need to also update test coverage for that (I haven't properly considered whether I agree with your logic yet, but happy to consider + get opinions of others in a new issue).

Automatically closed - issue fixed for 2 weeks with no activity.