Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-62709

core_message\task\migrate_message_data silently ignores database write error

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 3.5
    • Fix Version/s: 3.5.1
    • Component/s: Messages
    • Labels:

      Description

      After upgrading a test server to 3.5 from 3.3 and testing out the user messaging feature I noticed warnings about some sort of migration of messages to a new format. Researching further I discovered that the adhoc task core_message\task\migrate_message_data was running but failing with the following output (the JIRA text editor isn't letting me properly format so hopefully it comes through alright)

      Execute adhoc task: core_message\task\migrate_message_data
      ... started 11:49:29. Current memory use 34.9MB.
      ... used 25 dbqueries
      ... used 0.045833110809326 seconds
      Adhoc task failed: core_message\task\migrate_message_data,error/Task failed.
      Backtrace:
      * line 185 of /lib/cronlib.php: call to core_message\task\migrate_message_data->execute()
      * line 74 of /lib/cronlib.php: call to cron_run_inner_adhoc_task()
      * line 61 of /admin/cli/cron.php: call to cron_run()

      Checking in the task_adhoc table shows me a single entry for my user with a faildelay value indicating it will try again later. Re-running the cron script repeats the error output.

      Debugging this led me to this.

      You can see that any exception while writing to the database is silently ignored. In my case I was indeed getting an error during the migrate_data() call:

      Column 'fullmessageformat' cannot be null
      INSERT INTO notifications (useridfrom,useridto,subject,fullmessage,fullmessageformat,fullmessagehtml,smallmessage,component,eventtype,contexturl,contexturlname,timeread,timecreated) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?)
       
      ...snip...

      The underlying null field value that is causing the error may be due to one of my internal custom plugins not being compatible with the migration process, but in my opinion there should never, ever be a case of just outright ignoring an exception like this. It should be logged at the every least, especially since it's a \Throwable meaning even low-level type PHP errors would get covered up without the site admin having any idea why or any clear error messages to go by.

      I've developed a simple patch for it and will be linking to it shortly.

        Attachments

          Activity

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Jul/18