Moodle
  1. Moodle
  2. MDL-11893

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

    Details

    • Type: Improvement Improvement
    • Status: 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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            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
            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
            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
            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
            Petr Skoda 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 Skoda 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
            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
            Petr Skoda added a comment -

            code reviewed, closing - thanks everybody!

            Show
            Petr Skoda added a comment - code reviewed, closing - thanks everybody!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: