[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>

typo3/sysext/scheduler/Classes/Command/SchedulerCommand.php

@@ -30,11 +30,6 @@
  */
 class SchedulerCommand extends Command
 {
-    /**
-     * @var bool
-     */
-    protected $hasTask = true;
-
     /**
      * @var Scheduler
      */
@@ -122,8 +117,7 @@ public function execute(InputInterface $input, OutputInterface $output)
 
         $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 @@ protected function getTask(int $taskUid)
     /**
      * 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 @@ protected function loopTasks()
      *
      * 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);

typo3/sysext/scheduler/Classes/Scheduler.php

@@ -305,11 +305,11 @@ public function saveTask(AbstractTask $task)
      * 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 @@ public function fetchTask($uid = 0)
         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);