Skip to content

Commit

Permalink
ProcessJobExecutor: stdout output is captured and converted to notice
Browse files Browse the repository at this point in the history
  • Loading branch information
mabar committed Jan 12, 2024
1 parent bc08826 commit 5bd4e0c
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 13 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- prints job's `timeZone` parameter
- adds `--timezone` (`-tz`) option to show execution times in specified timezone
- adds `-n` shortcut for `--next` option
- `RunJobCommand`
- stdout is caught in a buffer and printed to output in a standardized manner (to above job result by default and
into key `stdout` in case `--json` option is used)

### Changed

Expand All @@ -54,7 +57,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- 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)
- if stderr output is produced, an exception is still thrown (explaining unexpected stderr instead of a job
failure)
- stdout output is captured and converted to notice (with strict error handler it will still cause an exception,
but will not block execution of other jobs)
- `ManagedScheduler`
- acquired job locks are scoped just to their id - changing run frequency or job name will not make process loose
the lock
Expand Down
40 changes: 30 additions & 10 deletions src/Command/RunJobCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Throwable;
use function json_decode;
use function json_encode;
use function ob_end_clean;
use function ob_get_clean;
use function ob_start;
use const JSON_PRETTY_PRINT;
use const JSON_THROW_ON_ERROR;

Expand Down Expand Up @@ -53,13 +57,23 @@ protected function execute(InputInterface $input, OutputInterface $output): int
{
$json = $input->getOption('json');
$params = $input->getOption('parameters');
$summary = $this->scheduler->runJob(
$input->getArgument('id'),
!$input->getOption('no-force'),
$params === null
? null
: RunParameters::fromArray(json_decode($params, true, 512, JSON_THROW_ON_ERROR)),
);

ob_start(static fn () => null);
try {
$summary = $this->scheduler->runJob(
$input->getArgument('id'),
!$input->getOption('no-force'),
$params === null
? null
: RunParameters::fromArray(json_decode($params, true, 512, JSON_THROW_ON_ERROR)),

Check warning on line 68 in src/Command/RunJobCommand.php

View workflow job for this annotation

GitHub Actions / Test for mutants (ubuntu-latest, 8.1)

Escaped Mutant for Mutator "DecrementInteger": --- Original +++ New @@ @@ $params = $input->getOption('parameters'); ob_start(static fn() => null); try { - $summary = $this->scheduler->runJob($input->getArgument('id'), !$input->getOption('no-force'), $params === null ? null : RunParameters::fromArray(json_decode($params, true, 512, JSON_THROW_ON_ERROR))); + $summary = $this->scheduler->runJob($input->getArgument('id'), !$input->getOption('no-force'), $params === null ? null : RunParameters::fromArray(json_decode($params, true, 511, JSON_THROW_ON_ERROR))); $stdout = ($tmp = ob_get_clean()) === false ? '' : $tmp; } catch (Throwable $e) { ob_end_clean();

Check warning on line 68 in src/Command/RunJobCommand.php

View workflow job for this annotation

GitHub Actions / Test for mutants (ubuntu-latest, 8.1)

Escaped Mutant for Mutator "IncrementInteger": --- Original +++ New @@ @@ $params = $input->getOption('parameters'); ob_start(static fn() => null); try { - $summary = $this->scheduler->runJob($input->getArgument('id'), !$input->getOption('no-force'), $params === null ? null : RunParameters::fromArray(json_decode($params, true, 512, JSON_THROW_ON_ERROR))); + $summary = $this->scheduler->runJob($input->getArgument('id'), !$input->getOption('no-force'), $params === null ? null : RunParameters::fromArray(json_decode($params, true, 513, JSON_THROW_ON_ERROR))); $stdout = ($tmp = ob_get_clean()) === false ? '' : $tmp; } catch (Throwable $e) { ob_end_clean();
);

$stdout = ($tmp = ob_get_clean()) === false ? '' : $tmp;
} catch (Throwable $e) {
ob_end_clean();

throw $e;
}

if ($summary === null) {
if ($json) {
Expand All @@ -72,8 +86,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

if ($json) {
$this->renderJobAsJson($summary, $output);
$this->renderJobAsJson($summary, $stdout, $output);
} else {
if ($stdout !== '') {
$output->writeln($stdout);
}

$this->renderJob($summary, $this->getTerminalWidth(), $output);
}

Expand All @@ -82,10 +100,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
: self::SUCCESS;
}

private function renderJobAsJson(JobSummary $summary, OutputInterface $output): void
private function renderJobAsJson(JobSummary $summary, string $stdout, OutputInterface $output): void
{
$jobData = $summary->toArray() + ['stdout' => $stdout];

$output->writeln(
json_encode($summary->toArray(), JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT),
json_encode($jobData, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT),
);
}

Expand Down
24 changes: 24 additions & 0 deletions src/Executor/ProcessJobExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@
use Orisai\Scheduler\Status\RunSummary;
use Psr\Clock\ClockInterface;
use Symfony\Component\Process\Process;
use Throwable;
use function assert;
use function is_array;
use function json_decode;
use function json_encode;
use function trigger_error;
use function trim;
use const E_USER_NOTICE;
use const JSON_THROW_ON_ERROR;
use const PHP_BINARY;

Expand Down Expand Up @@ -109,6 +112,17 @@ public function runJobs(
continue;
}

$stdout = $decoded['stdout'];
if ($stdout !== '') {
try {
$this->triggerUnexpectedStdout($execution, $stdout);
} catch (Throwable $e) {
$suppressedExceptions[] = $e;

continue;
}
}

if ($errorOutput !== '') {
$suppressedExceptions[] = $this->createStderrFail(
$execution,
Expand Down Expand Up @@ -204,4 +218,14 @@ private function createStderrFail(Process $execution, string $errorOutput): JobP
->withMessage($message);
}

private function triggerUnexpectedStdout(Process $execution, string $stdout): void
{
$message = Message::create()
->withContext("Running job via command {$execution->getCommandLine()}")
->withProblem('Job subprocess produced unsupported stdout output.')
->with('stdout', $stdout);

trigger_error($message->toString(), E_USER_NOTICE);
}

}
5 changes: 5 additions & 0 deletions tests/Doubles/CallbackList.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ public function errorJob(): void
throw new Error('test');
}

public function echoingJob(): void
{
echo 'output';
}

public function __invoke(): void
{
// Noop
Expand Down
74 changes: 73 additions & 1 deletion tests/Unit/Command/RunJobCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,79 @@ public function testJson(): void
"result": {
"end": "61.000000",
"state": "done"
}
},
"stdout": ""
}

MSG,
implode(
PHP_EOL,
array_map(
static fn (string $s): string => rtrim($s),
explode(PHP_EOL, preg_replace('~\R~u', PHP_EOL, $tester->getDisplay())),
),
),
);
self::assertSame($command::SUCCESS, $code);
}

public function testEchoingJob(): void
{
$clock = new FrozenClock(1, new DateTimeZone('Europe/Prague'));
$scheduler = new SimpleScheduler(null, null, null, $clock);

$cbs = new CallbackList();
$scheduler->addJob(
new CallbackJob(Closure::fromCallable([$cbs, 'echoingJob'])),
new CronExpression('* * * * *'),
);

$command = new RunJobCommand($scheduler);
$tester = new CommandTester($command);

putenv('COLUMNS=80');

$code = $tester->execute([
'id' => 0,
]);

self::assertSame(
<<<'MSG'
output
1970-01-01 01:00:01 Running [0] Tests\Orisai\Scheduler\Doubles\CallbackList::echoingJob() 0ms DONE

MSG,
implode(
PHP_EOL,
array_map(
static fn (string $s): string => rtrim($s),
explode(PHP_EOL, preg_replace('~\R~u', PHP_EOL, $tester->getDisplay())),
),
),
);
self::assertSame($command::SUCCESS, $code);

$code = $tester->execute([
'id' => 0,
'--json' => true,
]);

self::assertSame(
<<<'MSG'
{
"info": {
"id": 0,
"name": "Tests\\Orisai\\Scheduler\\Doubles\\CallbackList::echoingJob()",
"expression": "* * * * *",
"repeatAfterSeconds": 0,
"runSecond": 0,
"start": "1.000000"
},
"result": {
"end": "1.000000",
"state": "done"
},
"stdout": "output"
}

MSG,
Expand Down
18 changes: 18 additions & 0 deletions tests/Unit/SchedulerProcessSetup.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,24 @@ public static function createWithStderrJob(): ManagedScheduler
return new ManagedScheduler($jobManager, null, null, $executor, $clock);
}

public static function createWithStdoutJob(): ManagedScheduler
{
$jobManager = new SimpleJobManager();
$clock = new FrozenClock(1);
$executor = new ProcessJobExecutor($clock);
$executor->setExecutable(__DIR__ . '/scheduler-process-binary-with-stdout-job.php');

$jobManager->addJob(
new CallbackJob(static function (): void {
// STDOUT ignores output buffers and is pointless to test
echo ' echo ';
}),
new CronExpression('* * * * *'),
);

return new ManagedScheduler($jobManager, null, null, $executor, $clock);
}

/**
* @param Closure(Throwable, JobInfo, JobResult): (void)|null $errorHandler
*/
Expand Down
82 changes: 82 additions & 0 deletions tests/Unit/SimpleSchedulerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Cron\CronExpression;
use DateTimeImmutable;
use DateTimeZone;
use ErrorException;
use Exception;
use Generator;
use Orisai\Clock\FrozenClock;
Expand All @@ -31,6 +32,9 @@
use Tests\Orisai\Scheduler\Doubles\TestLockFactory;
use Throwable;
use function rtrim;
use function set_error_handler;
use const E_ALL;
use const E_USER_NOTICE;

final class SimpleSchedulerTest extends TestCase
{
Expand Down Expand Up @@ -1067,6 +1071,84 @@ public function testProcessJobStderr(): void
}
}

/**
* @runInSeparateProcess
*/
public function testProcessJobStdout(): void
{
$errors = [];

set_error_handler(
static function (int $errno, string $errstr) use (&$errors): bool {
$errors[] = [$errno, $errstr];

return true;
},
E_ALL,
);

$scheduler = SchedulerProcessSetup::createWithStdoutJob();
$summary = $scheduler->run();

self::assertCount(1, $summary->getJobSummaries());
self::assertCount(1, $errors);
foreach ($errors as [$code, $message]) {
self::assertSame(E_USER_NOTICE, $code);
self::assertStringMatchesFormat(
<<<'MSG'
Context: Running job via command %a
Problem: Job subprocess produced unsupported stdout output.
stdout: echo%c
MSG,
$message,
);
}
}

/**
* @runInSeparateProcess
*/
public function testProcessJobStdoutWithStrictErrorHandler(): void
{
set_error_handler(
static function (int $errno, string $errstr): void {
throw new ErrorException($errstr, $errno);
},
E_ALL,
);

$scheduler = SchedulerProcessSetup::createWithStdoutJob();

$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(ErrorException::class, $suppressed);
self::assertStringMatchesFormat(
<<<'MSG'
Context: Running job via command %a
Problem: Job subprocess produced unsupported stdout output.
stdout: echo%c
MSG,
$suppressed->getMessage(),
);
}
}

public function testProcessAfterRunEvent(): void
{
$scheduler = SchedulerProcessSetup::createEmpty();
Expand Down
17 changes: 17 additions & 0 deletions tests/Unit/scheduler-process-binary-with-stdout-job.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php declare(strict_types = 1);

use Orisai\Scheduler\Command\RunJobCommand;
use Symfony\Component\Console\Application;
use Tests\Orisai\Scheduler\Unit\SchedulerProcessSetup;

require_once __DIR__ . '/../../vendor/autoload.php';

$application = new Application();
$scheduler = SchedulerProcessSetup::createWithStdoutJob();

$command = new RunJobCommand($scheduler);

$application = new Application();
$application->addCommands([$command]);

$application->run();
2 changes: 1 addition & 1 deletion tools/phpstan.baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ parameters:

-
message: "#^Parameter \\#2 \\$str of function explode expects string, string\\|null given\\.$#"
count: 2
count: 4
path: ../tests/Unit/Command/RunJobCommandTest.php

-
Expand Down
1 change: 1 addition & 0 deletions tools/phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
beStrictAboutOutputDuringTests="true"
cacheResultFile="../var/tools/PHPUnit/results.dat"
colors="true"
convertNoticesToExceptions="false"
failOnRisky="true"
failOnWarning="true"
stderr="true"
Expand Down

0 comments on commit 5bd4e0c

Please sign in to comment.