From 50c5bdb8d2f15a4807afd1235d1e0ed742cc7648 Mon Sep 17 00:00:00 2001
From: Markus Klein <markus.klein@typo3.org>
Date: Thu, 23 Feb 2023 13:07:10 +0100
Subject: [PATCH] [BUGFIX] Output errors to CLI in scheduler:run command
Whenever a scheduler task fails during execution
via CLI `scheduler:run`, the error message is
now always written to stderr.
This gives the task executor (e.g. cron) a chance
to identify any failures, if even stdout is
redirected to /dev/null.
The task-loop is adapted to avoid (ab)using
exceptions for control-flow (for loop-abortion).
It instead now uses nullables to signal that
not finding a next-task is expected behavior,
not an exception.
Also the exit code of the command is adapted
to switch to 1 in case at least one error
occured. (This is important in case the cronjob
is executed as systemd-timer)
Resolves: #87806
Releases: main, 11.5
Change-Id: I0b55bab7fcd916209114d9e3e07f54c7243ce9a5
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/78220
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benjamin Franzke <benjaminfranzke@gmail.com>
Reviewed-by: Benjamin Franzke <benjaminfranzke@gmail.com>
---
.../Classes/Command/SchedulerCommand.php | 39 +++++++------------
typo3/sysext/scheduler/Classes/Scheduler.php | 6 +--
2 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/typo3/sysext/scheduler/Classes/Command/SchedulerCommand.php b/typo3/sysext/scheduler/Classes/Command/SchedulerCommand.php
index 64a502c86514..a207b3d65388 100644
--- a/typo3/sysext/scheduler/Classes/Command/SchedulerCommand.php
+++ b/typo3/sysext/scheduler/Classes/Command/SchedulerCommand.php
@@ -30,11 +30,6 @@ use TYPO3\CMS\Scheduler\Task\AbstractTask;
*/
class SchedulerCommand extends Command
{
- /**
- * @var bool
- */
- protected $hasTask = true;
-
/**
* @var Scheduler
*/
@@ -122,8 +117,7 @@ Call it like this: typo3/sysext/core/bin/typo3 scheduler:run --task=13 -f')
$this->forceExecution = (bool)$input->getOption('force');
$this->stopTasks = $this->shouldStopTasks((bool)$input->getOption('stop'));
- $this->loopTasks();
- return 0;
+ return $this->loopTasks() ? 0 : 1;
}
/**
@@ -184,38 +178,37 @@ Call it like this: typo3/sysext/core/bin/typo3 scheduler:run --task=13 -f')
/**
* Execute tasks in loop that are ready to execute
*/
- protected function loopTasks()
+ protected function loopTasks(): bool
{
+ $hasError = false;
do {
+ $task = null;
// Try getting the next task and execute it
// If there are no more tasks to execute, an exception is thrown by \TYPO3\CMS\Scheduler\Scheduler::fetchTask()
try {
$task = $this->fetchNextTask();
+ if ($task === null) {
+ break;
+ }
try {
$this->executeOrStopTask($task);
} catch (\Exception $e) {
- if ($this->io->isVerbose()) {
- $this->io->warning($e->getMessage());
- }
+ $this->io->getErrorStyle()->error($e->getMessage());
+ $hasError = true;
// We ignore any exception that may have been thrown during execution,
// as this is a background process.
// The exception message has been recorded to the database anyway
continue;
}
- } catch (\OutOfBoundsException $e) {
- if ($this->io->isVeryVerbose()) {
- $this->io->writeln($e->getMessage());
- }
- $this->hasTask = !empty($this->overwrittenTaskList);
} catch (\UnexpectedValueException $e) {
- if ($this->io->isVerbose()) {
- $this->io->warning($e->getMessage());
- }
+ $this->io->getErrorStyle()->error($e->getMessage());
+ $hasError = true;
continue;
}
- } while ($this->hasTask);
+ } while ($task !== null);
// Record the run in the system registry
$this->scheduler->recordLastRun();
+ return !$hasError;
}
/**
@@ -224,18 +217,16 @@ Call it like this: typo3/sysext/core/bin/typo3 scheduler:run --task=13 -f')
*
* Without the --task option we ask the scheduler for the next task with pending execution.
*
- * @return AbstractTask
- * @throws \OutOfBoundsException When there are no more tasks to execute.
* @throws \UnexpectedValueException When no task is found by the provided UID or the task is not marked for execution.
*/
- protected function fetchNextTask(): AbstractTask
+ protected function fetchNextTask(): ?AbstractTask
{
if ($this->overwrittenTaskList === null) {
return $this->scheduler->fetchTask();
}
if (count($this->overwrittenTaskList) === 0) {
- throw new \OutOfBoundsException('No more tasks to execute', 1547675594);
+ return null;
}
$taskUid = (int)array_shift($this->overwrittenTaskList);
diff --git a/typo3/sysext/scheduler/Classes/Scheduler.php b/typo3/sysext/scheduler/Classes/Scheduler.php
index 1afa55349a01..24bfde3500f9 100644
--- a/typo3/sysext/scheduler/Classes/Scheduler.php
+++ b/typo3/sysext/scheduler/Classes/Scheduler.php
@@ -305,11 +305,11 @@ class Scheduler implements SingletonInterface
* If there are no due tasks the method throws an exception.
*
* @param int $uid Primary key of a task
- * @return Task\AbstractTask The fetched task object
+ * @return Task\AbstractTask|null The fetched task object
* @throws \OutOfBoundsException
* @throws \UnexpectedValueException
*/
- public function fetchTask($uid = 0)
+ public function fetchTask($uid = 0): ?AbstractTask
{
$connectionPool = GeneralUtility::makeInstance(ConnectionPool::class);
$queryBuilder = $connectionPool->getQueryBuilderForTable('tx_scheduler_task');
@@ -352,7 +352,7 @@ class Scheduler implements SingletonInterface
if (empty($row)) {
if (empty($uid)) {
// No uid was passed and no overdue task was found
- throw new \OutOfBoundsException('No (more) tasks available for execution', 1247827244);
+ return null;
}
// Although a uid was passed, no task with given was found
throw new \OutOfBoundsException('No task with id ' . $uid . ' found', 1422044826);
--
GitLab