magemojo / m2-ce-cron

Magento 2 cron project to fix bugs and common cron issues.
https://magemojo.com
MIT License
165 stars 45 forks source link

Undefined index: method in Model/Schedule.php #95

Closed jaxtheking closed 4 years ago

jaxtheking commented 4 years ago

A while ago this module started to error out every night around 02:00. As a result of the error, the scheduling of new jobs stopped and it had to be restarted manually. The error was: Undefined index: method in .../vendor/magemojo/m2-ce-cron/Model/Schedule.php on line 375.

I finally tracked down and resolved the issue. It was the analytics_collect_data job. Its config looked like this:

$jobconfig: Array
(
    [schedule] => 00 02 * * *
    [name] => analytics_collect_data
    [group] => default
    [consumers] => 
)

The problem was that while I had disabled (intentionally) the Magento_Analytics module, the flags 'analytics/subscription/enabled' and 'crontab/default/jobs/analytics_collect_data/schedule/cron_expr' were still on in the core_config_data table due to a previous bug in disabling the module. I re-enabled the module, properly turned off Advanced Reporting, disabled the module again.

I mainly wanted to share this with others who may be experiencing this specific issue, but at the same time I'd like to propose (what I think is) an improvement to the error handling in the Schedule class:

/**
 * Replace method and instance values in the stub proc to be executed
 *
 * @return string
 */
public function prepareStub($jobconfig, $stub, $scheduleid) {
  try {
    $code = trim($stub);
    $code = str_replace('<<basedir>>',$this->basedir,$code);
    $code = str_replace('<<method>>',$jobconfig["method"],$code);
    $code = str_replace('<<instance>>',$jobconfig["instance"],$code);
    $code = str_replace('<<scheduleid>>',$scheduleid,$code);
    $code = str_replace('<<name>>',$jobconfig["name"],$code);
    return $code;
  } catch (\Exception $e) {
      $this->_logger->error($e->getMessage(), $jobconfig);
  }
}

Basically if we were to wrap the current code in a try/catch block, the loop would recover from an error. Also, adding the $jobconfig array as a context parameter to the logger message, one would know right away from the logs which cron job was the cause of the error - whatever this may be. Maybe the try/catch should be placed prior to calling prepareStub (in setJobStatus)... I don't know.

Another solution may be to issue a warning in the logs if the 'method' index does not exist. This way we'd avoid the module to error out while still warning developers.

I'm no Magento wiz, so I'm interested to know what the author of the module thinks. Regards, Luca

gnuzealot commented 4 years ago

Fixed in 1.3.3