diff --git a/CHANGELOG.md b/CHANGELOG.md index 393cd70..1ffd320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and ## [Unreleased] +### Fixed + +- **`ConsoleChannel` sent error-level lines to stdout when handed a command's output.** + `Illuminate\Console\Command::run()` wraps the terminal output in an `OutputStyle`, which is not a + `ConsoleOutputInterface`, so the `instanceof` check never found the error stream. The channel now + unwraps the style first, a local copy of `laranail/console`'s `ErrorOutput::unwrap()` since this + module carries no `laranail/*` dependency. Nothing in the package passes it an `OutputStyle` + today; an application that injected `$this->output` from a command did. `require` now names + `illuminate/console` for `OutputStyle`; `laravel/framework` already replaces it, so an + application installs nothing new. + ### Changed - `require` declares what `src/` uses directly: `illuminate/filesystem` and `illuminate/log` diff --git a/composer.json b/composer.json index a1d7b7e..f5c24ab 100644 --- a/composer.json +++ b/composer.json @@ -38,6 +38,7 @@ "php": "^8.3 || ^8.4 || ^8.5", "illuminate/bus": "^13.0", "illuminate/cache": "^13.0", + "illuminate/console": "^13.0", "illuminate/contracts": "^13.0", "illuminate/database": "^13.0", "illuminate/filesystem": "^13.0", diff --git a/src/Channels/ConsoleChannel.php b/src/Channels/ConsoleChannel.php index 90db6c2..e6af622 100644 --- a/src/Channels/ConsoleChannel.php +++ b/src/Channels/ConsoleChannel.php @@ -5,6 +5,7 @@ namespace Simtabi\Laranail\Notifications\Channels; use Throwable; +use Illuminate\Console\OutputStyle; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Output\ConsoleOutputInterface; use Simtabi\Laranail\Notifications\DataTransferObjects\NotificationMessage; @@ -16,6 +17,12 @@ * legacy adapter delegated to `laranail/console-tools`, which is removed here. * Output goes to an injected {@see OutputInterface} when one is available, * otherwise straight to STDOUT/STDERR via `fwrite` with a safe fallback. + * + * Error-level lines go to the error stream. A command's output is an {@see OutputStyle}, which + * `Illuminate\Console\Command::run()` wraps around the terminal's `ConsoleOutput` and which is not + * a {@see ConsoleOutputInterface}, so it is unwrapped first. This is a local copy of + * `laranail/console`'s `Tools\Support\ErrorOutput::unwrap()`, kept local because this module + * carries no `laranail/*` dependency. */ class ConsoleChannel extends AbstractNotificationChannel { @@ -39,8 +46,10 @@ public function send(NotificationMessage $message): bool try { if ($this->output !== null) { - if ($isError && $this->output instanceof ConsoleOutputInterface) { - $this->output->getErrorOutput()->writeln($line); + $console = $this->output instanceof OutputStyle ? $this->output->getOutput() : $this->output; + + if ($isError && $console instanceof ConsoleOutputInterface) { + $console->getErrorOutput()->writeln($line); } else { $this->output->writeln($line); } diff --git a/tests/Unit/ConsoleChannelStreamTest.php b/tests/Unit/ConsoleChannelStreamTest.php new file mode 100644 index 0000000..e6e9b22 --- /dev/null +++ b/tests/Unit/ConsoleChannelStreamTest.php @@ -0,0 +1,93 @@ +stderr = new BufferedOutput; + } + + public function getErrorOutput(): OutputInterface + { + return $this->stderr; + } + + public function setErrorOutput(OutputInterface $error): void + { + $this->stderr = $error; + } + + public function section(): ConsoleSectionOutput + { + throw new LogicException('Sections are not used by these tests.'); + } + + public function stderr(): string + { + return $this->stderr instanceof BufferedOutput ? $this->stderr->fetch() : ''; + } +} + +/** + * Stream routing when the channel is handed a command's output. + * + * `Illuminate\Console\Command::run()` wraps the terminal output in an `OutputStyle`, which is not a + * `ConsoleOutputInterface`, so a bare `instanceof` check sends error-level lines to stdout. + */ +final class ConsoleChannelStreamTest extends TestCase +{ + public function test_an_error_reaches_stderr_through_an_output_style(): void + { + $console = new SplitBufferedOutput; + $channel = new ConsoleChannel([], new OutputStyle(new ArrayInput([]), $console)); + + $this->assertTrue($channel->send(new NotificationMessage('disk full', level: 'error'))); + + $this->assertSame('', $console->fetch()); + $this->assertStringContainsString('[ERROR] disk full', $console->stderr()); + } + + public function test_an_info_line_stays_on_stdout_through_an_output_style(): void + { + $console = new SplitBufferedOutput; + $channel = new ConsoleChannel([], new OutputStyle(new ArrayInput([]), $console)); + + $this->assertTrue($channel->send(new NotificationMessage('deployed', level: 'info'))); + + $this->assertStringContainsString('[INFO] deployed', $console->fetch()); + $this->assertSame('', $console->stderr()); + } + + public function test_an_error_reaches_stderr_on_a_bare_console_output(): void + { + $console = new SplitBufferedOutput; + $channel = new ConsoleChannel([], $console); + + $this->assertTrue($channel->send(new NotificationMessage('disk full', level: 'error'))); + + $this->assertSame('', $console->fetch()); + $this->assertStringContainsString('[ERROR] disk full', $console->stderr()); + } +}