diff --git a/system/CLI/BaseCommand.php b/system/CLI/BaseCommand.php index d5da586fcf1f..d28a36c2eb97 100644 --- a/system/CLI/BaseCommand.php +++ b/system/CLI/BaseCommand.php @@ -150,7 +150,9 @@ protected function showError(Throwable $e) $message = $e->getMessage(); $config = config(Exceptions::class); - require $config->errorViewPath . '/cli/error_exception.php'; + CLI::redirectToStderr(static function () use ($exception, $message, $config): void { + require $config->errorViewPath . '/cli/error_exception.php'; + }); } /** diff --git a/system/CLI/CLI.php b/system/CLI/CLI.php index a7c711bbe51d..b2e5a189f708 100644 --- a/system/CLI/CLI.php +++ b/system/CLI/CLI.php @@ -136,6 +136,8 @@ class CLI */ protected static ?InputOutput $io = null; + protected static bool $stdoutToStderr = false; + /** * Static "constructor". * @@ -1138,9 +1140,44 @@ public static function table(array $tbody, array $thead = []) */ protected static function fwrite($handle, string $string) { + if (static::$stdoutToStderr && $handle === STDOUT) { + $handle = STDERR; + } + static::$io->fwrite($handle, $string); } + /** + * Runs the callback with its STDOUT writes sent to STDERR. + * + * @param callable(): void $callback + * + * @internal + */ + public static function redirectToStderr(callable $callback): void + { + if (! is_cli()) { + $callback(); + + return; + } + + $stdoutToStderr = static::$stdoutToStderr; + $isColored = static::$isColored; + $lastWrite = static::$lastWrite; + + static::$stdoutToStderr = true; + static::$isColored = static::hasColorSupport(STDERR); + + try { + $callback(); + } finally { + static::$stdoutToStderr = $stdoutToStderr; + static::$isColored = $isColored; + static::$lastWrite = $lastWrite; + } + } + /** * Testing purpose only * @@ -1148,13 +1185,14 @@ protected static function fwrite($handle, string $string) */ public static function reset(): void { - static::$initialized = false; - static::$segments = []; - static::$options = []; - static::$lastWrite = null; - static::$height = null; - static::$width = null; - static::$isColored = static::hasColorSupport(STDOUT); + static::$initialized = false; + static::$segments = []; + static::$options = []; + static::$lastWrite = null; + static::$height = null; + static::$width = null; + static::$isColored = static::hasColorSupport(STDOUT); + static::$stdoutToStderr = false; static::resetInputOutput(); } diff --git a/system/Debug/ExceptionHandler.php b/system/Debug/ExceptionHandler.php index 61ba7cc82b2a..1958984dbb22 100644 --- a/system/Debug/ExceptionHandler.php +++ b/system/Debug/ExceptionHandler.php @@ -15,6 +15,7 @@ use Closure; use CodeIgniter\API\ResponseTrait; +use CodeIgniter\CLI\CLI; use CodeIgniter\Exceptions\PageNotFoundException; use CodeIgniter\HTTP\CLIRequest; use CodeIgniter\HTTP\Exceptions\HTTPException; @@ -121,7 +122,11 @@ public function handle( } // Displays the HTML or CLI error code. - $this->render($exception, $statusCode, $viewFile); + if ($request instanceof IncomingRequest) { + $this->render($exception, $statusCode, $viewFile); + } else { + CLI::redirectToStderr(fn () => $this->render($exception, $statusCode, $viewFile)); + } if (! service('environment')->isTesting()) { exit($exitCode); // @codeCoverageIgnore diff --git a/tests/system/CLI/AbstractCommandTest.php b/tests/system/CLI/AbstractCommandTest.php index cf3db24fdf7a..853e67a81e4f 100644 --- a/tests/system/CLI/AbstractCommandTest.php +++ b/tests/system/CLI/AbstractCommandTest.php @@ -26,6 +26,7 @@ use CodeIgniter\Commands\Help; use CodeIgniter\Exceptions\LogicException; use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\Filters\CITestStreamFilter; use CodeIgniter\Test\Mock\MockInputOutput; use CodeIgniter\Test\StreamFilterTrait; use Config\App; @@ -234,6 +235,8 @@ public function testRenderThrowable(): void { $command = new AppAboutCommand(new Commands()); + CITestStreamFilter::removeOutputFilter(); + $this->assertSame(EXIT_ERROR, $command->bomb()); $this->assertStringContainsString('[CodeIgniter\CLI\Exceptions\CLIException]', $this->getStreamFilterBuffer()); $this->assertStringContainsString('Invalid "background" color: "Background".', $this->getStreamFilterBuffer()); diff --git a/tests/system/CLI/BaseCommandTest.php b/tests/system/CLI/BaseCommandTest.php index bf9faf2fae28..6546ea909376 100644 --- a/tests/system/CLI/BaseCommandTest.php +++ b/tests/system/CLI/BaseCommandTest.php @@ -17,6 +17,7 @@ use CodeIgniter\CodeIgniter; use CodeIgniter\Log\Logger; use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\Filters\CITestStreamFilter; use CodeIgniter\Test\StreamFilterTrait; use PHPUnit\Framework\Attributes\After; use PHPUnit\Framework\Attributes\Before; @@ -89,6 +90,8 @@ public function testShowError(): void { $command = new AppInfo(single_service('logger'), single_service('commands')); + CITestStreamFilter::removeOutputFilter(); + $this->assertSame(1, $command->bomb()); $this->assertStringContainsString('[CodeIgniter\CLI\Exceptions\CLIException]', $this->getStreamFilterBuffer()); $this->assertStringContainsString('Invalid "background" color: "Background".', $this->getStreamFilterBuffer()); diff --git a/tests/system/CLI/CLITest.php b/tests/system/CLI/CLITest.php index 7f6e21ba40b0..233724114c3f 100644 --- a/tests/system/CLI/CLITest.php +++ b/tests/system/CLI/CLITest.php @@ -649,6 +649,96 @@ public function testMixedWriteError(): void ); } + public function testRedirectToStderrRoutesStdoutWritesOnlyWhileTheCallbackRuns(): void + { + $io = new class () extends InputOutput { + /** + * @var list + */ + public array $writes = []; + + public function fwrite($handle, string $string): void + { + $this->writes[] = [$handle === STDERR ? 'STDERR' : 'STDOUT', $string]; + } + }; + CLI::setInputOutput($io); + + CLI::print('before'); + + CLI::redirectToStderr(static function (): void { + CLI::write('inside'); + }); + + try { + CLI::redirectToStderr(static function (): void { + throw new RuntimeException('Callback failed.'); + }); + } catch (RuntimeException) { + } + + CLI::write('after'); + + $this->assertSame([ + ['STDOUT', 'before'], + ['STDERR', "\ninside\n"], + ['STDOUT', "\nafter\n"], + ], $io->writes); + } + + public function testRedirectToStderrDoesNotRedirectOutsideCli(): void + { + $io = new class () extends InputOutput { + /** + * @var list + */ + public array $writes = []; + + public function fwrite($handle, string $string): void + { + $this->writes[] = [$handle === STDERR ? 'STDERR' : 'STDOUT', $string]; + } + }; + CLI::setInputOutput($io); + + is_cli(false); + + try { + CLI::redirectToStderr(static function (): void { + CLI::write('inside'); + }); + } finally { + is_cli(true); + } + + $this->assertSame([['STDOUT', "\ninside\n"]], $io->writes); + } + + public function testRedirectToStderrUsesTheColorSupportOfStderr(): void + { + $superglobals = service('superglobals'); + $noColor = $superglobals->server('NO_COLOR'); + + (new ReflectionProperty(CLI::class, 'isColored'))->setValue(null, true); + $superglobals->setServer('NO_COLOR', '1'); + + try { + CLI::redirectToStderr(static function (): void { + CLI::write('inside', 'red'); + }); + } finally { + if ($noColor === null) { + $superglobals->unsetServer('NO_COLOR'); + } else { + $superglobals->setServer('NO_COLOR', $noColor); + } + } + + CLI::write('after', 'red'); + + $this->assertSame("\ninside\n\n\033[0;31mafter\033[0m\n", $this->getStreamFilterBuffer()); + } + public function testErrorForeground(): void { CLI::error('test', 'purple'); diff --git a/tests/system/Debug/ExceptionHandlerTest.php b/tests/system/Debug/ExceptionHandlerTest.php index 5f263edfc4a7..5ed9fa462a0c 100644 --- a/tests/system/Debug/ExceptionHandlerTest.php +++ b/tests/system/Debug/ExceptionHandlerTest.php @@ -21,6 +21,7 @@ use CodeIgniter\HTTP\SiteURI; use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\Filters\CITestStreamFilter; use CodeIgniter\Test\IniTestTrait; use CodeIgniter\Test\StreamFilterTrait; use Config\App; @@ -253,6 +254,21 @@ public function testHandleCLIPageNotFoundException(): void $this->resetStreamFilterBuffer(); } + public function testHandleCLIExceptionWritesToStderr(): void + { + $request = Services::clirequest(null, false); + /** @var Response $response */ + $response = service('response', null, false); + $response->pretend(); + + CITestStreamFilter::removeOutputFilter(); + + $this->handler->handle(new RuntimeException('Something broke.'), $request, $response, 500, EXIT_ERROR); + + $this->assertStringContainsString('[' . RuntimeException::class . ']', $this->getStreamFilterBuffer()); + $this->assertStringContainsString('Something broke.', $this->getStreamFilterBuffer()); + } + public function testMaskSensitiveData(): void { $maskSensitiveData = self::getPrivateMethodInvoker($this->handler, 'maskSensitiveData'); diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 5c20fcbbd54e..b925dbbf119a 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -25,6 +25,9 @@ Behavior Changes - The static ``Boot::initializeConsole()`` method no longer handles the display of the console header. This is now handled within ``Console::run()``. If you have overridden ``Boot::initializeConsole()``, you should remove any code related to displaying the console header, as this is now the responsibility of the ``Console`` class. +- **CLI:** Uncaught exceptions in ``spark``, and errors shown with ``BaseCommand::showError()`` or ``AbstractCommand::renderThrowable()`` (as ``migrate`` and + ``db:seed`` do), are now printed to STDERR instead of STDOUT. The exit code is unchanged. + See :ref:`upgrade-480-cli-error-output`. - **Commands:** The ``-h`` option to ``routes`` command which was mapped previously to the ``--sort-by-handler`` option is now removed. Use ``--sort-by-handler`` instead to sort the routes by handler when running the ``routes`` command. - **Commands:** The ``filter:check`` command now requires the HTTP method argument to be uppercase (e.g., ``spark filter:check GET /`` instead of ``spark filter:check get /``). diff --git a/user_guide_src/source/cli/cli_commands.rst b/user_guide_src/source/cli/cli_commands.rst index 6b09ca0f120c..8c954dbcd43d 100644 --- a/user_guide_src/source/cli/cli_commands.rst +++ b/user_guide_src/source/cli/cli_commands.rst @@ -127,7 +127,7 @@ be familiar with when creating your own commands. It also has a :doc:`Logger my.log``), errors no longer +reach that file. Redirect STDERR as well to keep them (``php spark my:command > my.log 2>&1``). + +A custom CLI error view that writes with ``echo`` instead of the ``CLI`` class still prints to +STDOUT. + Uploaded File Move Return Type ============================== diff --git a/utils/phpstan-baseline/arguments.count.neon b/utils/phpstan-baseline/arguments.count.neon index 999a1d99310f..254e39c2f397 100644 --- a/utils/phpstan-baseline/arguments.count.neon +++ b/utils/phpstan-baseline/arguments.count.neon @@ -1,10 +1,10 @@ -# total 4 errors +# total 6 errors parameters: ignoreErrors: - message: '#^Function is_cli invoked with 1 parameter, 0 required\.$#' - count: 2 + count: 4 path: ../../tests/system/CLI/CLITest.php - diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon index 4c09d5998045..20de678702e1 100644 --- a/utils/phpstan-baseline/loader.neon +++ b/utils/phpstan-baseline/loader.neon @@ -1,4 +1,4 @@ -# total 30 errors +# total 32 errors includes: - arguments.count.neon