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
    • Rank:
      37070

      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

          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 Škoda 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 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 Škoda added a comment -

          code reviewed, closing - thanks everybody!

          Show
          Petr Škoda 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: