#220 - Throw a ProcessFailedException instead of a FatalError on the stop error strategy

CHANGELOG.md

@@ -20,6 +20,7 @@ Latest
 * [#207](https://github.com/cleverage/process-bundle/issues/207) Fix JsonStreamReaderTask / JsonStreamWriterTask: throw an explicit `\UnexpectedValueException` when a line decodes to a scalar, create the missing parent directory when writing. Update documentation, add tests.
 * [#208](https://github.com/cleverage/process-bundle/issues/208) Fix FolderBrowserTask, InputFolderBrowserTask, CsvReaderTask and LineReaderTask (and their `Input*` variants): reset the state at the end of the iteration, so that a following input is read from its beginning. Update documentation, add tests.
 * [#201](https://github.com/cleverage/process-bundle/issues/201) Fix minor defects: error messages of TransformerTrait, RulesTransformer and ExpressionLanguageMapTransformer, useless `setRequired()` in ImplodeTransformer and SprintfTransformer, stray namespace in TrimTransformer, wrong or missing docblocks. Add tests.
+* [#220](https://github.com/cleverage/process-bundle/issues/220) Fix the stop error strategy: throw a `ProcessFailedException` (with the original exception as `previous`) instead of a `FatalError`, so that the command exits with a non-zero code when a process fails and `ProcessLauncherTask` detects failed sub-processes. Update documentation, add tests.
 
 ## Deprecated
 * [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0.

docs/01-quick_start.md

@@ -262,6 +262,9 @@ Once everything is working fine, you may want to automate your processes. The st
 0 */2 * * * /path/to/project/bin/console cleverage:process:execute <my_process> --env=prod
 ```
 
+When a process fails, the command exits with a non-zero code, so failures can be detected by the scheduler (or by a
+CI job, a supervisor...).
+
 This bundle does not store any execution history in database. Process and task logs are sent to dedicated Monolog
 channels (`cleverage_process`, `cleverage_process_task` and `cleverage_process_transformer`, see
 [logging](03-custom_tasks.md#logging)), so you can route them to any handler. Each record is enriched with the

docs/04-advanced_workflow.md

@@ -23,9 +23,8 @@ When a process is executed (with the `cleverage:process:execute` command or with
 
 If an exception is thrown at any point, the `cleverage_process.fail` event is dispatched and the exception is rethrown.
 Note that a task error handled by the `stop` strategy does not surface as the original exception: the process manager
-throws a new `Symfony\Component\ErrorHandler\Error\FatalError` (an `\Error`, not an `\Exception`), whose message
-contains the process code, the task code and the original message; the original exception is not attached as
-`previous` (it is only available in the task error log record).
+throws a `CleverAge\ProcessBundle\Exception\ProcessFailedException` (a `\RuntimeException`), whose message contains
+the process code, the task code and the original message. The original exception is available with `getPrevious()`.
 
 ### Executing a process from PHP
 
@@ -111,7 +110,7 @@ on the `cleverage_process_task` channel with the `log_level` of the task (`criti
 default):
 - `skip`: the current output is dropped, and the process continues with the next input (e.g. the next line of a CSV
   file)
-- `stop`: the whole process stops and fails (a `FatalError` is thrown by the process manager, see
+- `stop`: the whole process stops and fails (a `ProcessFailedException` is thrown by the process manager, see
   [process execution flow](#process-execution-flow))
 
 Before applying the strategy, the task input is sent to the tasks listed in `error_outputs` (unless the task already

docs/06-testing.md

@@ -102,10 +102,10 @@ public function execute(string $processCode, mixed $input = null, array $context
 
 `$input` is given to the process `entry_point`, `$context` is the same as the `--context` option of the command, and
 the returned value is the last output of the process `end_point` (`null` if there is none). If the process fails, the
-exception is rethrown; but a task error handled by the `stop` error strategy is thrown as a new
-`Symfony\Component\ErrorHandler\Error\FatalError` (an `\Error`), which only keeps the original message (the original
-exception is not attached as `previous`). Test it with `expectException(FatalError::class)` and
-`expectExceptionMessageMatches()` rather than with the original exception class.
+exception is rethrown; but a task error handled by the `stop` error strategy is thrown as a
+`CleverAge\ProcessBundle\Exception\ProcessFailedException`, the original exception being available with
+`getPrevious()`. Test it with `expectException(ProcessFailedException::class)`, and check the original exception with
+`getPrevious()` if needed.
 
 ```yaml
 # config/packages/test/clever_age_process.yaml

src/Exception/ProcessFailedException.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Exception;
+
+/**
+ * Thrown when a process is stopped by a task error (error strategy "stop").
+ *
+ * The original exception is available with getPrevious().
+ */
+class ProcessFailedException extends \RuntimeException implements ProcessExceptionInterface
+{
+    public static function create(string $processCode, string $taskCode, \Throwable $previous): self
+    {
+        $errorStr = "Process {$processCode} has failed during process {$taskCode}";
+        $errorStr .= " with message: '{$previous->getMessage()}'.\n";
+
+        return new self($errorStr, 0, $previous);
+    }
+}

src/Manager/ProcessManager.php

@@ -18,6 +18,7 @@
 use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
 use CleverAge\ProcessBundle\Event\ProcessEvent;
 use CleverAge\ProcessBundle\Exception\InvalidProcessConfigurationException;
+use CleverAge\ProcessBundle\Exception\ProcessFailedException;
 use CleverAge\ProcessBundle\Logger\ProcessLogger;
 use CleverAge\ProcessBundle\Logger\TaskLogger;
 use CleverAge\ProcessBundle\Model\BlockingTaskInterface;
@@ -30,7 +31,6 @@
 use CleverAge\ProcessBundle\Model\TaskInterface;
 use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry;
 use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\ErrorHandler\Error\FatalError;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -290,12 +290,7 @@ protected function process(TaskConfiguration $taskConfiguration, int $executionF
             if ($state->isStopped()) {
                 $exception = $state->getException();
                 if ($exception instanceof \Throwable) {
-                    $m = "Process {$state->getProcessConfiguration()
-                        ->getCode()} has failed";
-                    $m .= " during process {$state->getTaskConfiguration()
-                        ->getCode()}";
-                    $m .= " with message: '{$exception->getMessage()}'.\n";
-                    throw new FatalError($m, -1, ['file' => $exception->getFile(), 'line' => $exception->getLine(), 'type' => 500, 'message' => $exception->getMessage()]);
+                    throw ProcessFailedException::create($state->getProcessConfiguration()->getCode(), $state->getTaskConfiguration()->getCode(), $exception);
                 }
 
                 return;

tests/Manager/ProcessManagerStopStrategyTest.php

@@ -0,0 +1,145 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Manager;
+
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Event\ProcessEvent;
+use CleverAge\ProcessBundle\Exception\ProcessFailedException;
+use CleverAge\ProcessBundle\Logger\AbstractLogger;
+use CleverAge\ProcessBundle\Logger\ProcessLogger;
+use CleverAge\ProcessBundle\Logger\TaskLogger;
+use CleverAge\ProcessBundle\Manager\ProcessManager;
+use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\ProcessBundle\Model\TaskInterface;
+use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry;
+use CleverAge\ProcessBundle\Task\ConstantOutputTask;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\EventDispatcher\EventDispatcher;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(ProcessManager::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(ProcessFailedException::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessEvent::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractLogger::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfigurationRegistry::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ConstantOutputTask::class)]
+class ProcessManagerStopStrategyTest extends TestCase
+{
+    public function testStopStrategyThrowsAProcessFailedExceptionWithTheOriginalException(): void
+    {
+        $originalException = new \LogicException('Something went wrong', 42);
+        $processManager = $this->createProcessManager($originalException);
+
+        try {
+            $processManager->execute('test.process');
+            self::fail('The process should have failed');
+        } catch (\Exception $exception) { // An \Exception, not an \Error: catchable with catch (\Exception)
+            self::assertInstanceOf(ProcessFailedException::class, $exception);
+            self::assertSame(
+                "Process test.process has failed during process fail with message: 'Something went wrong'.\n",
+                $exception->getMessage()
+            );
+            self::assertSame($originalException, $exception->getPrevious());
+        }
+    }
+
+    public function testSkipStrategyDoesNotThrow(): void
+    {
+        $processManager = $this->createProcessManager(new \LogicException('Something went wrong'), 'skip');
+
+        self::assertNull($processManager->execute('test.process'));
+    }
+
+    private function createProcessManager(\Throwable $exception, string $errorStrategy = 'stop'): ProcessManager
+    {
+        $failingTask = new class($exception) implements TaskInterface {
+            public function __construct(
+                private readonly \Throwable $exception,
+            ) {
+            }
+
+            public function execute(ProcessState $state): void
+            {
+                throw $this->exception;
+            }
+        };
+
+        $container = new Container();
+        $container->set('test.constant', new ConstantOutputTask());
+        $container->set('test.failing', $failingTask);
+
+        $registry = new ProcessConfigurationRegistry(
+            [
+                'test.process' => [
+                    'options' => [],
+                    'entry_point' => null,
+                    'end_point' => null,
+                    'description' => '',
+                    'help' => '',
+                    'public' => true,
+                    'tasks' => [
+                        'entry' => $this->createTaskConfiguration('@test.constant', ['output' => 'value'], ['fail']),
+                        'fail' => $this->createTaskConfiguration('@test.failing', [], [], $errorStrategy),
+                    ],
+                ],
+            ],
+            'stop'
+        );
+
+        return new ProcessManager(
+            $container,
+            new ProcessLogger(new NullLogger()),
+            new TaskLogger(new NullLogger()),
+            $registry,
+            new ContextualOptionResolver(),
+            new EventDispatcher(),
+        );
+    }
+
+    /**
+     * @param array<string, mixed> $options
+     * @param list<string>         $outputs
+     *
+     * @return array<string, mixed>
+     */
+    private function createTaskConfiguration(
+        string $service,
+        array $options = [],
+        array $outputs = [],
+        ?string $errorStrategy = null,
+    ): array {
+        return [
+            'service' => $service,
+            'options' => $options,
+            'description' => '',
+            'help' => '',
+            'outputs' => $outputs,
+            'errors' => [],
+            'error_outputs' => [],
+            'error_strategy' => $errorStrategy,
+            'log_level' => null,
+        ];
+    }
+}