From dbbe5e6f562d7e0a450a8087da9d6a14ed3951b8 Mon Sep 17 00:00:00 2001 From: DariusIII Date: Fri, 7 Aug 2026 08:47:47 +0200 Subject: [PATCH] Update processrunner. Closes #1867 --- app/Services/ForkingService.php | 2 +- app/Services/Runners/BaseRunner.php | 22 ++++-- .../Unit/Services/Runners/BaseRunnerTest.php | 70 +++++++++++++++++++ 3 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 tests/Unit/Services/Runners/BaseRunnerTest.php diff --git a/app/Services/ForkingService.php b/app/Services/ForkingService.php index ca8b9c2fd..51076e1ec 100644 --- a/app/Services/ForkingService.php +++ b/app/Services/ForkingService.php @@ -305,7 +305,7 @@ class ForkingService protected function executeCommand(string $command): string { $process = Process::fromShellCommandline($command); - $process->setTimeout(1800); + $process->setTimeout((int) config('nntmux.multiprocessing_max_child_time', 1800)); $process->run(function ($type, $buffer) { if ($type === Process::ERR) { echo $buffer; diff --git a/app/Services/Runners/BaseRunner.php b/app/Services/Runners/BaseRunner.php index ff52b61dd..0a98c1acc 100644 --- a/app/Services/Runners/BaseRunner.php +++ b/app/Services/Runners/BaseRunner.php @@ -5,6 +5,8 @@ declare(strict_types=1); namespace App\Services\Runners; use Illuminate\Support\Facades\Log; +use RuntimeException; +use Symfony\Component\Process\Exception\ProcessTimedOutException; use Symfony\Component\Process\Process; abstract class BaseRunner @@ -141,12 +143,20 @@ abstract class BaseRunner protected function executeCommand(string $command): string { $process = Process::fromShellCommandline($command); - $process->setTimeout(1800); - $process->run(function ($type, $buffer) { - if ($type === Process::ERR) { - echo $buffer; - } - }); + $process->setTimeout($this->concurrencyTimeout()); + + try { + $process->run(function ($type, $buffer) { + if ($type === Process::ERR) { + echo $buffer; + } + }); + } catch (ProcessTimedOutException $e) { + // Rethrow as RuntimeException: Laravel's Concurrency ProcessDriver cannot + // reconstruct ProcessTimedOutException (its constructor requires a Process + // object), which would otherwise surface as an unrelated TypeError. + throw new RuntimeException($e->getMessage()); + } return $process->getOutput(); } diff --git a/tests/Unit/Services/Runners/BaseRunnerTest.php b/tests/Unit/Services/Runners/BaseRunnerTest.php new file mode 100644 index 000000000..855d213f1 --- /dev/null +++ b/tests/Unit/Services/Runners/BaseRunnerTest.php @@ -0,0 +1,70 @@ +assertSame('hello', trim($runner->runCommand('echo hello'))); + } + + #[Test] + public function execute_command_throws_runtime_exception_with_clear_message_on_timeout(): void + { + config(['nntmux.concurrency_timeout' => 1]); + + $runner = new BaseRunnerTestDouble; + + try { + $runner->runCommand('sleep 5'); + $this->fail('Expected RuntimeException was not thrown'); + } catch (RuntimeException $e) { + // Laravel's Concurrency ProcessDriver cannot reconstruct + // ProcessTimedOutException, so executeCommand() must surface a + // RuntimeException carrying the original timeout message instead. + $this->assertStringContainsString('exceeded the timeout', $e->getMessage()); + } + } + + #[Test] + public function concurrency_timeout_prefers_concurrency_timeout_config(): void + { + config(['nntmux.concurrency_timeout' => 60]); + config(['nntmux.multiprocessing_max_child_time' => 42]); + + $this->assertSame(60, (new BaseRunnerTestDouble)->timeout()); + } + + #[Test] + public function concurrency_timeout_falls_back_to_multiprocessing_max_child_time(): void + { + config(['nntmux.concurrency_timeout' => null]); + config(['nntmux.multiprocessing_max_child_time' => 42]); + + $this->assertSame(42, (new BaseRunnerTestDouble)->timeout()); + } +} + +class BaseRunnerTestDouble extends BaseRunner +{ + public function runCommand(string $command): string + { + return $this->executeCommand($command); + } + + public function timeout(): int + { + return $this->concurrencyTimeout(); + } +}