134 invalid task hardening

[matthewhilton]

Closes #134

Closes #136

Changes

Removed use of task \core\task\manager.

It's much more reliable to just read the data directly from the database. This is because the manager class can throw errors, or the task class itself can throw them like described in the #134. The manager class also outputs debugging if the task is not found, which pollutes the output (unless we force disable debugging).

Its much simpler and cleaner to just read directly from the DB. We lose the ability to get the scheduled task name, but IMO this is not a big deal since you can just read it from the class.

Using the DB also has the advantage of being able to easily de-dup the tasks (For #136)

Added try/catch for check API errors

If a check class is broken or throws an exception, now we get a nice error message:

CRITICAL: Error scanning checks: coding_exception: Coding error detected, it must be fixed by a programmer: test in /var/www/moodle-311-heartbeat/admin/tool/heartbeat/classes/check/authcheck.php:60
Stack trace:
#0 /var/www/moodle-311-heartbeat/admin/tool/heartbeat/croncheck.php(315): tool_heartbeat\check\authcheck->get_result()
#1 {main}
 (Checked Oct 3 13:42:42)

This indicates something went really wrong, since ideally check classes should always be properly linked.

I'm still a little undecided whether to include just the message, or the full error (incl. stacktrace). I leaned towards full error so that you can tell what plugin/code is throwing the error.

Testing / example:

CRITICAL: Moodle task faildelay > 600 mins
SCHEDULED TASK: \core\task\cache_cron_task Delay: 86400
ADHOC TASK: \tool_dataprivacy\task\process_data_request_task Delay: 86400  (4 duplicates!!!)
ADHOC TASK: \tool_dataprivacy\task\initiate_data_request_task Delay: 86400 
 (Checked Oct 9 09:39:40)

[brendanheywood]

this all looks great, the only nit pick is adding line breaks to the sql after each comma

possibly you should also sort the tasks by cumulative faildelay

[matthewhilton]

this all looks great, the only nit pick is adding line breaks to the sql after each comma

Do you mean:

SELECT classname,
COUNT(*) count,
MAX(faildelay) faildelay
...

(if this is what you mean) IMO I prefer the select parameters all on the one line, with the SQL 'verb' always being at the start

possibly you should also sort the tasks by cumulative faildelay

I have added this:

WARNING: Moodle task faildelay > 60 mins
ADHOC TASK: \tool_dataprivacy\task\process_data_request_task Delay: 1000  (13 duplicates!!!)
ADHOC TASK: \tool_dataprivacy\task\initiate_data_request_task Delay: 6001 
 (Checked Oct 9 16:20:48)