From 267ae14cf98cc2b880f3597d147e2c65ea80fa4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Barto=C5=A1?= Date: Thu, 11 Jan 2024 21:57:18 +0100 Subject: [PATCH] ProcessJobExecutor: handles stdout and stderr separately --- CHANGELOG.md | 3 + src/Executor/ProcessJobExecutor.php | 64 +++++++++++++---- tests/Unit/SchedulerProcessSetup.php | 36 ++++++++++ tests/Unit/SimpleSchedulerTest.php | 72 ++++++++++++++++++- .../Unit/scheduler-process-binary-sdterr.php | 3 + ...heduler-process-binary-with-stderr-job.php | 17 +++++ 6 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 tests/Unit/scheduler-process-binary-sdterr.php create mode 100644 tests/Unit/scheduler-process-binary-with-stderr-job.php diff --git a/CHANGELOG.md b/CHANGELOG.md index faec2a8..2ff5725 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `ProcessJobExecutor` - uses microseconds instead of milliseconds for start and end times - better exception message in case subprocess call failed + - handles stdout and stderr separately + - stderr output does not make the job processing fail + - if stderr output is produced, an exception is still thrown (explaining unexpected stderr instead of a job failure) - `ManagedScheduler` - acquired job locks are scoped just to their id - changing run frequency or job name will not make process loose the lock diff --git a/src/Executor/ProcessJobExecutor.php b/src/Executor/ProcessJobExecutor.php index 074d756..df8de10 100644 --- a/src/Executor/ProcessJobExecutor.php +++ b/src/Executor/ProcessJobExecutor.php @@ -26,6 +26,7 @@ use function is_array; use function json_decode; use function json_encode; +use function trim; use const JSON_THROW_ON_ERROR; use const PHP_BINARY; @@ -86,26 +87,36 @@ public function runJobs( // Check running jobs foreach ($jobExecutions as $i => [$execution, $cronExpression]) { assert($execution instanceof Process); - if (!$execution->isRunning()) { - unset($jobExecutions[$i]); + if ($execution->isRunning()) { + continue; + } - $output = $execution->getOutput() . $execution->getErrorOutput(); + unset($jobExecutions[$i]); - try { - $decoded = json_decode($output, true, 512, JSON_THROW_ON_ERROR); - assert(is_array($decoded)); + $output = $execution->getOutput(); + $errorOutput = trim($execution->getErrorOutput()); - yield $jobSummaries[] = $this->createSummary($decoded, $cronExpression); - } catch (JsonException $e) { - $message = Message::create() - ->withContext("Running job via command {$execution->getCommandLine()}") - ->withProblem('Job subprocess failed.') - ->with('stdout + stderr (standard + error output)', $output); + try { + $decoded = json_decode($output, true, 512, JSON_THROW_ON_ERROR); + assert(is_array($decoded)); + } catch (JsonException $e) { + $suppressedExceptions[] = $this->createSubprocessFail( + $execution, + $output, + $errorOutput, + ); - $suppressedExceptions[] = JobProcessFailure::create() - ->withMessage($message); - } + continue; + } + + if ($errorOutput !== '') { + $suppressedExceptions[] = $this->createStderrFail( + $execution, + $errorOutput, + ); } + + yield $jobSummaries[] = $this->createSummary($decoded, $cronExpression); } // Nothing to do, wait @@ -170,4 +181,27 @@ private function createSummary(array $raw, CronExpression $cronExpression): JobS ); } + private function createSubprocessFail(Process $execution, string $output, string $errorOutput): JobProcessFailure + { + $message = Message::create() + ->withContext("Running job via command {$execution->getCommandLine()}") + ->withProblem('Job subprocess failed.') + ->with('stdout', trim($output)) + ->with('stderr', $errorOutput); + + return JobProcessFailure::create() + ->withMessage($message); + } + + private function createStderrFail(Process $execution, string $errorOutput): JobProcessFailure + { + $message = Message::create() + ->withContext("Running job via command {$execution->getCommandLine()}") + ->withProblem('Job subprocess produced stderr output.') + ->with('stderr', $errorOutput); + + return JobProcessFailure::create() + ->withMessage($message); + } + } diff --git a/tests/Unit/SchedulerProcessSetup.php b/tests/Unit/SchedulerProcessSetup.php index ac39d84..6286463 100644 --- a/tests/Unit/SchedulerProcessSetup.php +++ b/tests/Unit/SchedulerProcessSetup.php @@ -15,6 +15,8 @@ use Orisai\Scheduler\Status\JobResult; use Tests\Orisai\Scheduler\Doubles\CallbackList; use Throwable; +use function fwrite; +use const STDERR; final class SchedulerProcessSetup { @@ -65,6 +67,40 @@ public static function createWithThrowingJob(): ManagedScheduler return new ManagedScheduler($jobManager, null, null, $executor, $clock); } + public static function createWithStderr(): ManagedScheduler + { + $jobManager = new SimpleJobManager(); + $clock = new FrozenClock(1); + $executor = new ProcessJobExecutor($clock); + $executor->setExecutable(__DIR__ . '/scheduler-process-binary-sdterr.php'); + + $jobManager->addJob( + new CallbackJob(static function (): void { + // Just forces executable to run + }), + new CronExpression('* * * * *'), + ); + + return new ManagedScheduler($jobManager, null, null, $executor, $clock); + } + + public static function createWithStderrJob(): ManagedScheduler + { + $jobManager = new SimpleJobManager(); + $clock = new FrozenClock(1); + $executor = new ProcessJobExecutor($clock); + $executor->setExecutable(__DIR__ . '/scheduler-process-binary-with-stderr-job.php'); + + $jobManager->addJob( + new CallbackJob(static function (): void { + fwrite(STDERR, ' job error '); + }), + new CronExpression('* * * * *'), + ); + + return new ManagedScheduler($jobManager, null, null, $executor, $clock); + } + /** * @param Closure(Throwable, JobInfo, JobResult): (void)|null $errorHandler */ diff --git a/tests/Unit/SimpleSchedulerTest.php b/tests/Unit/SimpleSchedulerTest.php index d66caaf..0e0144c 100644 --- a/tests/Unit/SimpleSchedulerTest.php +++ b/tests/Unit/SimpleSchedulerTest.php @@ -991,7 +991,77 @@ public function testProcessExecutorWithDefaultExecutable(): void Context: Running job via command '/usr/bin/php%f' 'bin/console' 'scheduler:run-job' '%a' '--json' '--parameters' '{"second":%d}' Problem: Job subprocess failed. -stdout + stderr (standard + error output): Could not open input file: bin/console +stdout: Could not open input file: bin/console +stderr: +MSG, + rtrim($suppressed->getMessage()), + ); + } + } + + public function testProcessStderr(): void + { + $scheduler = SchedulerProcessSetup::createWithStderr(); + + $e = null; + try { + $scheduler->run(); + } catch (RunFailure $e) { + // Handled bellow + } + + self::assertNotNull($e); + self::assertStringStartsWith( + <<<'MSG' +Run failed +Suppressed errors: +MSG, + $e->getMessage(), + ); + + self::assertNotSame([], $e->getSuppressed()); + foreach ($e->getSuppressed() as $suppressed) { + self::assertInstanceOf(JobProcessFailure::class, $suppressed); + self::assertStringMatchesFormat( + <<<'MSG' +Context: Running job via command '/usr/bin/php%f'%c%w'%a/tests/Unit/scheduler-process-binary-sdterr.php'%c%w'scheduler:run-job' '%a' '--json' '--parameters' '{"second":%d}' +Problem: Job subprocess failed. +stdout:%c +stderr: error +MSG, + rtrim($suppressed->getMessage()), + ); + } + } + + public function testProcessJobStderr(): void + { + $scheduler = SchedulerProcessSetup::createWithStderrJob(); + + $e = null; + try { + $scheduler->run(); + } catch (RunFailure $e) { + // Handled bellow + } + + self::assertNotNull($e); + self::assertStringStartsWith( + <<<'MSG' +Run failed +Suppressed errors: +MSG, + $e->getMessage(), + ); + + self::assertNotSame([], $e->getSuppressed()); + foreach ($e->getSuppressed() as $suppressed) { + self::assertInstanceOf(JobProcessFailure::class, $suppressed); + self::assertStringMatchesFormat( + <<<'MSG' +Context: Running job via command '/usr/bin/php%f'%c%w'%a/tests/Unit/scheduler-process-binary-with-stderr-job.php'%c%w'scheduler:run-job' '%a' '--json' '--parameters' '{"second":%d}' +Problem: Job subprocess produced stderr output. +stderr: job error MSG, rtrim($suppressed->getMessage()), ); diff --git a/tests/Unit/scheduler-process-binary-sdterr.php b/tests/Unit/scheduler-process-binary-sdterr.php new file mode 100644 index 0000000..2bf6b5a --- /dev/null +++ b/tests/Unit/scheduler-process-binary-sdterr.php @@ -0,0 +1,3 @@ +addCommands([$command]); + +$application->run();