Moodle

add_to_log should check the return value of insert_record and send and flag erors to admin

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9.1
  • Component/s: Administration
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

When the disk fills up, the add_to_log() function is usually the first thing to fail.

We can alert the $CFG->supportemail address of the problem when insert fails.

Issue Links

This issue has been marked as being related by:
MDL-14100 Insert into log table failed at Saturday 29th of March 2008 10:11:43 AM. It is possible that your disk is full Critical Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Yu Zhang added a comment -

Added in 1.9 and 2.0

Show
Yu Zhang added a comment - Added in 1.9 and 2.0
Hide
Permalink
Martin Dougiamas added a comment -

Dongsheng, can you please alter this so that:

  • the insert query is added to the email so we have more information about the failure
  • when an email is sent it checks for a config flag:
    $lasttime = get_config('admin', 'inserterroremail');
  • if this value is less than one day ago then it does nothing.
  • if the value is more that one day ago then it sends the mail and resets the config flag set_config('inserterrormail', time(), 'admin');
Show
Martin Dougiamas added a comment - Dongsheng, can you please alter this so that:
  • the insert query is added to the email so we have more information about the failure
  • when an email is sent it checks for a config flag: $lasttime = get_config('admin', 'inserterroremail');
  • if this value is less than one day ago then it does nothing.
  • if the value is more that one day ago then it sends the mail and resets the config flag set_config('inserterrormail', time(), 'admin');
Hide
Permalink
Dongsheng Cai added a comment -

fixed in 1.9.1, and merged into head, please review.

Show
Dongsheng Cai added a comment - fixed in 1.9.1, and merged into head, please review.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Thanks for the patch Dongsheng.

I've detected some problems with it and I've committed a new version. It includes:

  • Fixed logic: If the 'inserterrormail' config variable didn't exist, email wasn't ever sent!
  • Renamed 'inserterrormail' to 'lastloginserterrormail'. More precise var name IMO.
  • Minor formatting changes in the email (just added a couple of linefeeds here and there).

Sent to 19_STABLE and HEAD. Changing QA to Petr

About the direct use of the mail() function.... it's a good idea although perhaps it would be ok to use some semaphore (static variable) to avoid recursion and then use the standard email_to_user() Moodle alternative. It handles better a lot of things.

For your consideration. In its current state, it's enough good to build the WEEKLY properly. Ciao

Show
Eloy Lafuente (stronk7) added a comment - Thanks for the patch Dongsheng. I've detected some problems with it and I've committed a new version. It includes:
  • Fixed logic: If the 'inserterrormail' config variable didn't exist, email wasn't ever sent!
  • Renamed 'inserterrormail' to 'lastloginserterrormail'. More precise var name IMO.
  • Minor formatting changes in the email (just added a couple of linefeeds here and there).
Sent to 19_STABLE and HEAD. Changing QA to Petr About the direct use of the mail() function.... it's a good idea although perhaps it would be ok to use some semaphore (static variable) to avoid recursion and then use the standard email_to_user() Moodle alternative. It handles better a lot of things. For your consideration. In its current state, it's enough good to build the WEEKLY properly. Ciao
Hide
Permalink
Petr Škoda (skodak) added a comment -

if (!$result and debugging()) { echo '<p>Error: Could not insert a new entry to the Moodle log</p>'; // Don't throw an error }

should be imo improved - DEBUG_ALL level I think

Show
Petr Škoda (skodak) added a comment - if (!$result and debugging()) { echo '<p>Error: Could not insert a new entry to the Moodle log</p>'; // Don't throw an error } should be imo improved - DEBUG_ALL level I think
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Good point. Change to proper use of debugging() - with DEBUG_ALL level. Thanks and ciao

Show
Eloy Lafuente (stronk7) added a comment - Good point. Change to proper use of debugging() - with DEBUG_ALL level. Thanks and ciao
Hide
Permalink
Petr Škoda (skodak) added a comment -

code reviewed, closing - thanks everybody!

Show
Petr Škoda (skodak) added a comment - code reviewed, closing - thanks everybody!

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.