From 9c5cccb9c41817be06c6322a32676a895c4a5037 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 16:47:04 +0200 Subject: [PATCH] fix(manager) #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. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/01-quick_start.md | 3 + docs/04-advanced_workflow.md | 7 +- docs/06-testing.md | 8 +- src/Exception/ProcessFailedException.php | 30 ++++ src/Manager/ProcessManager.php | 9 +- .../ProcessManagerStopStrategyTest.php | 145 ++++++++++++++++++ 7 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 src/Exception/ProcessFailedException.php create mode 100644 tests/Manager/ProcessManagerStopStrategyTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 237cb95b..126d73e3 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/docs/01-quick_start.md b/docs/01-quick_start.md index 4b7359b9..758d6456 100644 --- a/docs/01-quick_start.md +++ b/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 --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 diff --git a/docs/04-advanced_workflow.md b/docs/04-advanced_workflow.md index 77354d8c..f0876c09 100644 --- a/docs/04-advanced_workflow.md +++ b/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 diff --git a/docs/06-testing.md b/docs/06-testing.md index bf0fb5d0..ff8786e3 100644 --- a/docs/06-testing.md +++ b/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 diff --git a/src/Exception/ProcessFailedException.php b/src/Exception/ProcessFailedException.php new file mode 100644 index 00000000..09ec8df7 --- /dev/null +++ b/src/Exception/ProcessFailedException.php @@ -0,0 +1,30 @@ +getMessage()}'.\n"; + + return new self($errorStr, 0, $previous); + } +} diff --git a/src/Manager/ProcessManager.php b/src/Manager/ProcessManager.php index e561366f..a252e778 100644 --- a/src/Manager/ProcessManager.php +++ b/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; diff --git a/tests/Manager/ProcessManagerStopStrategyTest.php b/tests/Manager/ProcessManagerStopStrategyTest.php new file mode 100644 index 00000000..a2399dd6 --- /dev/null +++ b/tests/Manager/ProcessManagerStopStrategyTest.php @@ -0,0 +1,145 @@ +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 $options + * @param list $outputs + * + * @return array + */ + 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, + ]; + } +}