From a7c67c9ab24e7ab985b46b45ec79e104a4a653fd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 13 Apr 2017 20:34:34 +0200 Subject: [PATCH] [Console] Review console.ERROR related behavior --- .../Resources/config/console.xml | 2 +- src/Symfony/Component/Console/Application.php | 112 +++++++++--------- .../Component/Console/ConsoleEvents.php | 3 +- .../Console/Event/ConsoleErrorEvent.php | 63 +++------- .../Console/Event/ConsoleExceptionEvent.php | 12 +- .../Console/Event/ConsoleTerminateEvent.php | 2 +- ...xceptionListener.php => ErrorListener.php} | 4 +- .../Console/Tests/ApplicationTest.php | 30 +++-- ...ListenerTest.php => ErrorListenerTest.php} | 35 +++--- 9 files changed, 114 insertions(+), 149 deletions(-) rename src/Symfony/Component/Console/EventListener/{ExceptionListener.php => ErrorListener.php} (97%) rename src/Symfony/Component/Console/Tests/EventListener/{ExceptionListenerTest.php => ErrorListenerTest.php} (76%) diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml index 585622bca8379..350c2e20ef7a7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml @@ -6,7 +6,7 @@ - + diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index 668ed44df5a94..7d3f1c674e07f 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -119,46 +119,30 @@ public function run(InputInterface $input = null, OutputInterface $output = null $output = new ConsoleOutput(); } + if (null !== $this->dispatcher && $this->dispatcher->hasListeners(ConsoleEvents::EXCEPTION)) { + @trigger_error(sprintf('The "ConsoleEvents::EXCEPTION" event is deprecated since Symfony 3.3 and will be removed in 4.0. Listen to the "ConsoleEvents::ERROR" event instead.'), E_USER_DEPRECATED); + } + $this->configureIO($input, $output); try { $e = null; $exitCode = $this->doRun($input, $output); - } catch (\Exception $e) { - $exception = $e; - } catch (\Throwable $e) { - $exception = new FatalThrowableError($e); - } - - if (null !== $e && null !== $this->dispatcher) { - $event = new ConsoleErrorEvent($input, $output, $e, $e->getCode(), $this->runningCommand); - $this->dispatcher->dispatch(ConsoleEvents::ERROR, $event); - - $e = $event->getError(); - - if ($event->isErrorHandled()) { - $e = null; - $exitCode = 0; - } else { - if (!$e instanceof \Exception) { - throw $e; - } - $exitCode = $e->getCode(); - } - - $event = new ConsoleTerminateEvent($this->runningCommand, $input, $output, $exitCode); - $this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event); + } catch (\Exception $x) { + $e = $x; + } catch (\Throwable $x) { + $e = new FatalThrowableError($x); } if (null !== $e) { if (!$this->catchExceptions) { - throw $e; + throw $x; } if ($output instanceof ConsoleOutputInterface) { - $this->renderException($exception, $output->getErrorOutput()); + $this->renderException($e, $output->getErrorOutput()); } else { - $this->renderException($exception, $output); + $this->renderException($e, $output); } $exitCode = $e->getCode(); @@ -214,8 +198,26 @@ public function doRun(InputInterface $input, OutputInterface $output) $input = new ArrayInput(array('command' => $this->defaultCommand)); } - // the command name MUST be the first element of the input - $command = $this->find($name); + try { + $e = $this->runningCommand = null; + // the command name MUST be the first element of the input + $command = $this->find($name); + } catch (\Exception $e) { + } catch (\Throwable $e) { + } + if (null !== $e) { + if (null !== $this->dispatcher) { + $event = new ConsoleErrorEvent($input, $output, $e); + $this->dispatcher->dispatch(ConsoleEvents::ERROR, $event); + $e = $event->getError(); + + if (0 === $event->getExitCode()) { + return 0; + } + } + + throw $e; + } $this->runningCommand = $command; $exitCode = $this->doRunCommand($command, $input, $output); @@ -864,13 +866,7 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI } if (null === $this->dispatcher) { - try { - return $command->run($input, $output); - } catch (\Exception $e) { - throw $e; - } catch (\Throwable $e) { - throw new FatalThrowableError($e); - } + return $command->run($input, $output); } // bind before the console.command event, so the listeners have access to input options/arguments @@ -882,37 +878,45 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI } $event = new ConsoleCommandEvent($command, $input, $output); - $this->dispatcher->dispatch(ConsoleEvents::COMMAND, $event); - if ($event->commandShouldRun()) { - try { - $e = null; + try { + $e = null; + $this->dispatcher->dispatch(ConsoleEvents::COMMAND, $event); + + if ($event->commandShouldRun()) { $exitCode = $command->run($input, $output); - } catch (\Exception $x) { - $e = $x; - } catch (\Throwable $x) { - $e = new FatalThrowableError($x); + } else { + $exitCode = ConsoleCommandEvent::RETURN_CODE_DISABLED; } - - if (null !== $e) { - $event = new ConsoleExceptionEvent($command, $input, $output, $e, $e->getCode(), false); + } catch (\Exception $e) { + } catch (\Throwable $e) { + } + if (null !== $e) { + if ($this->dispatcher->hasListeners(ConsoleEvents::EXCEPTION)) { + $x = $e instanceof \Exception ? $e : new FatalThrowableError($e); + $event = new ConsoleExceptionEvent($command, $input, $output, $x, $x->getCode()); $this->dispatcher->dispatch(ConsoleEvents::EXCEPTION, $event); - if ($e !== $event->getException()) { - @trigger_error('The "console.exception" event is deprecated since version 3.3 and will be removed in 4.0. Use the "console.error" event instead.', E_USER_DEPRECATED); - - $x = $e = $event->getException(); + if ($x !== $event->getException()) { + $e = $event->getException(); } + } + $event = new ConsoleErrorEvent($input, $output, $e, $command); + $this->dispatcher->dispatch(ConsoleEvents::ERROR, $event); + $e = $event->getError(); - throw $x; + if (0 === $exitCode = $event->getExitCode()) { + $e = null; } - } else { - $exitCode = ConsoleCommandEvent::RETURN_CODE_DISABLED; } $event = new ConsoleTerminateEvent($command, $input, $output, $exitCode); $this->dispatcher->dispatch(ConsoleEvents::TERMINATE, $event); + if (null !== $e) { + throw $e; + } + return $event->getExitCode(); } diff --git a/src/Symfony/Component/Console/ConsoleEvents.php b/src/Symfony/Component/Console/ConsoleEvents.php index 57036733a2d52..7f7d4a3f28ff0 100644 --- a/src/Symfony/Component/Console/ConsoleEvents.php +++ b/src/Symfony/Component/Console/ConsoleEvents.php @@ -55,8 +55,7 @@ final class ConsoleEvents const EXCEPTION = 'console.exception'; /** - * The ERROR event occurs when an uncaught exception appears or - * a throwable error. + * The ERROR event occurs when an uncaught exception or error appears. * * This event allows you to deal with the exception/error or * to modify the thrown exception. diff --git a/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php b/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php index 1e5bffae7a380..49edb723d212d 100644 --- a/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php +++ b/src/Symfony/Component/Console/Event/ConsoleErrorEvent.php @@ -15,31 +15,22 @@ use Symfony\Component\Console\Exception\InvalidArgumentException; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\Debug\Exception\FatalThrowableError; /** * Allows to handle throwables thrown while running a command. * * @author Wouter de Jong */ -class ConsoleErrorEvent extends ConsoleExceptionEvent +final class ConsoleErrorEvent extends ConsoleEvent { private $error; - private $handled = false; + private $exitCode; - public function __construct(InputInterface $input, OutputInterface $output, $error, $exitCode, Command $command = null) + public function __construct(InputInterface $input, OutputInterface $output, $error, Command $command = null) { - if (!$error instanceof \Throwable && !$error instanceof \Exception) { - throw new InvalidArgumentException(sprintf('The error passed to ConsoleErrorEvent must be an instance of \Throwable or \Exception, "%s" was passed instead.', is_object($error) ? get_class($error) : gettype($error))); - } - - $exception = $error; - if (!$error instanceof \Exception) { - $exception = new FatalThrowableError($error); - } - parent::__construct($command, $input, $output, $exception, $exitCode, false); + parent::__construct($command, $input, $output); - $this->error = $error; + $this->setError($error); } /** @@ -67,46 +58,26 @@ public function setError($error) } /** - * Marks the error/exception as handled. - * - * If it is not marked as handled, the error/exception will be displayed in - * the command output. - */ - public function markErrorAsHandled() - { - $this->handled = true; - } - - /** - * Whether the error/exception is handled by a listener or not. - * - * If it is not yet handled, the error/exception will be displayed in the - * command output. + * Sets the exit code. * - * @return bool - */ - public function isErrorHandled() - { - return $this->handled; - } - - /** - * @deprecated Since version 3.3, to be removed in 4.0. Use getError() instead + * @param int $exitCode The command exit code */ - public function getException() + public function setExitCode($exitCode) { - @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent::getError() instead.', __METHOD__), E_USER_DEPRECATED); + $this->exitCode = (int) $exitCode; - return parent::getException(); + $r = new \ReflectionProperty($this->error, 'code'); + $r->setAccessible(true); + $r->setValue($this->error, $this->exitCode); } /** - * @deprecated Since version 3.3, to be removed in 4.0. Use setError() instead + * Gets the exit code. + * + * @return int The command exit code */ - public function setException(\Exception $exception) + public function getExitCode() { - @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent::setError() instead.', __METHOD__), E_USER_DEPRECATED); - - parent::setException($exception); + return null !== $this->exitCode ? $this->exitCode : ($this->error->getCode() ?: 1); } } diff --git a/src/Symfony/Component/Console/Event/ConsoleExceptionEvent.php b/src/Symfony/Component/Console/Event/ConsoleExceptionEvent.php index 09571cd318dc2..a31797fa35038 100644 --- a/src/Symfony/Component/Console/Event/ConsoleExceptionEvent.php +++ b/src/Symfony/Component/Console/Event/ConsoleExceptionEvent.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Console\Event; +@trigger_error(sprintf('The "%s" class is deprecated since version 3.3 and will be removed in 4.0. Use the ConsoleErrorEvent instead.', ConsoleExceptionEvent::class), E_USER_DEPRECATED); + use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -20,22 +22,18 @@ * * @author Fabien Potencier * - * @deprecated ConsoleExceptionEvent is deprecated since version 3.3 and will be removed in 4.0. Use ConsoleErrorEvent instead. + * @deprecated since version 3.3, to be removed in 4.0. Use ConsoleErrorEvent instead. */ class ConsoleExceptionEvent extends ConsoleEvent { private $exception; private $exitCode; - public function __construct(Command $command = null, InputInterface $input, OutputInterface $output, \Exception $exception, $exitCode, $deprecation = true) + public function __construct(Command $command, InputInterface $input, OutputInterface $output, \Exception $exception, $exitCode) { - if ($deprecation) { - @trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use the ConsoleErrorEvent instead.', __CLASS__), E_USER_DEPRECATED); - } - parent::__construct($command, $input, $output); - $this->exception = $exception; + $this->setException($exception); $this->exitCode = (int) $exitCode; } diff --git a/src/Symfony/Component/Console/Event/ConsoleTerminateEvent.php b/src/Symfony/Component/Console/Event/ConsoleTerminateEvent.php index 80bc2fa9eb989..b6a5d7c0dc48c 100644 --- a/src/Symfony/Component/Console/Event/ConsoleTerminateEvent.php +++ b/src/Symfony/Component/Console/Event/ConsoleTerminateEvent.php @@ -29,7 +29,7 @@ class ConsoleTerminateEvent extends ConsoleEvent */ private $exitCode; - public function __construct(Command $command = null, InputInterface $input, OutputInterface $output, $exitCode) + public function __construct(Command $command, InputInterface $input, OutputInterface $output, $exitCode) { parent::__construct($command, $input, $output); diff --git a/src/Symfony/Component/Console/EventListener/ExceptionListener.php b/src/Symfony/Component/Console/EventListener/ErrorListener.php similarity index 97% rename from src/Symfony/Component/Console/EventListener/ExceptionListener.php rename to src/Symfony/Component/Console/EventListener/ErrorListener.php index 94bbc22fac41f..8e35d97dfd489 100644 --- a/src/Symfony/Component/Console/EventListener/ExceptionListener.php +++ b/src/Symfony/Component/Console/EventListener/ErrorListener.php @@ -12,9 +12,9 @@ namespace Symfony\Component\Console\EventListener; use Psr\Log\LoggerInterface; -use Symfony\Component\Console\Event\ConsoleEvent; use Symfony\Component\Console\ConsoleEvents; use Symfony\Component\Console\Event\ConsoleErrorEvent; +use Symfony\Component\Console\Event\ConsoleEvent; use Symfony\Component\Console\Event\ConsoleTerminateEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -22,7 +22,7 @@ * @author James Halsall * @author Robin Chalas */ -class ExceptionListener implements EventSubscriberInterface +class ErrorListener implements EventSubscriberInterface { private $logger; diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php index 2c080a0b80a7d..f0dd72719b13d 100644 --- a/src/Symfony/Component/Console/Tests/ApplicationTest.php +++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php @@ -1045,7 +1045,7 @@ public function testRunAllowsErrorListenersToSilenceTheException() $dispatcher->addListener('console.error', function (ConsoleErrorEvent $event) { $event->getOutput()->write('silenced.'); - $event->markErrorAsHandled(); + $event->setExitCode(0); }); $dispatcher->addListener('console.command', function () { @@ -1063,7 +1063,7 @@ public function testRunAllowsErrorListenersToSilenceTheException() $tester = new ApplicationTester($application); $tester->run(array('command' => 'foo')); $this->assertContains('before.error.silenced.after.', $tester->getDisplay()); - $this->assertEquals(0, $tester->getStatusCode()); + $this->assertEquals(ConsoleCommandEvent::RETURN_CODE_DISABLED, $tester->getStatusCode()); } public function testConsoleErrorEventIsTriggeredOnCommandNotFound() @@ -1073,7 +1073,6 @@ public function testConsoleErrorEventIsTriggeredOnCommandNotFound() $this->assertNull($event->getCommand()); $this->assertInstanceOf(CommandNotFoundException::class, $event->getError()); $event->getOutput()->write('silenced command not found'); - $event->markErrorAsHandled(); }); $application = new Application(); @@ -1083,12 +1082,12 @@ public function testConsoleErrorEventIsTriggeredOnCommandNotFound() $tester = new ApplicationTester($application); $tester->run(array('command' => 'unknown')); $this->assertContains('silenced command not found', $tester->getDisplay()); - $this->assertEquals(0, $tester->getStatusCode()); + $this->assertEquals(1, $tester->getStatusCode()); } /** * @group legacy - * @expectedDeprecation The "console.exception" event is deprecated since version 3.3 and will be removed in 4.0. Use the "console.error" event instead. + * @expectedDeprecation The "ConsoleEvents::EXCEPTION" event is deprecated since Symfony 3.3 and will be removed in 4.0. Listen to the "ConsoleEvents::ERROR" event instead. */ public function testLegacyExceptionListenersAreStillTriggered() { @@ -1115,13 +1114,6 @@ public function testLegacyExceptionListenersAreStillTriggered() public function testRunWithError() { - if (method_exists($this, 'expectException')) { - $this->expectException('Exception'); - $this->expectExceptionMessage('dymerr'); - } else { - $this->setExpectedException('Exception', 'dymerr'); - } - $application = new Application(); $application->setAutoExit(false); $application->setCatchExceptions(false); @@ -1133,7 +1125,13 @@ public function testRunWithError() }); $tester = new ApplicationTester($application); - $tester->run(array('command' => 'dym')); + + try { + $tester->run(array('command' => 'dym')); + $this->fail('Error expected.'); + } catch (\Error $e) { + $this->assertSame('dymerr', $e->getMessage()); + } } /** @@ -1143,6 +1141,7 @@ public function testErrorIsRethrownIfNotHandledByConsoleErrorEvent() { $application = new Application(); $application->setAutoExit(false); + $application->setCatchExceptions(false); $application->setDispatcher(new EventDispatcher()); $application->register('dym')->setCode(function (InputInterface $input, OutputInterface $output) { @@ -1154,8 +1153,7 @@ public function testErrorIsRethrownIfNotHandledByConsoleErrorEvent() try { $tester->run(array('command' => 'dym')); $this->fail('->run() should rethrow PHP errors if not handled via ConsoleErrorEvent.'); - } catch (\Throwable $e) { - $this->assertInstanceOf('Error', $e); + } catch (\Error $e) { $this->assertSame($e->getMessage(), 'Class \'UnknownClass\' not found'); } } @@ -1378,7 +1376,7 @@ protected function getDispatcher($skipCommand = false) $event->getOutput()->writeln('after.'); if (!$skipCommand) { - $event->setExitCode(113); + $event->setExitCode(ConsoleCommandEvent::RETURN_CODE_DISABLED); } }); $dispatcher->addListener('console.error', function (ConsoleErrorEvent $event) { diff --git a/src/Symfony/Component/Console/Tests/EventListener/ExceptionListenerTest.php b/src/Symfony/Component/Console/Tests/EventListener/ErrorListenerTest.php similarity index 76% rename from src/Symfony/Component/Console/Tests/EventListener/ExceptionListenerTest.php rename to src/Symfony/Component/Console/Tests/EventListener/ErrorListenerTest.php index 295f75c067644..c857a97d0b9bc 100644 --- a/src/Symfony/Component/Console/Tests/EventListener/ExceptionListenerTest.php +++ b/src/Symfony/Component/Console/Tests/EventListener/ErrorListenerTest.php @@ -16,7 +16,7 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Event\ConsoleErrorEvent; use Symfony\Component\Console\Event\ConsoleTerminateEvent; -use Symfony\Component\Console\EventListener\ExceptionListener; +use Symfony\Component\Console\EventListener\ErrorListener; use Symfony\Component\Console\Input\ArgvInput; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\Input; @@ -24,36 +24,36 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -class ExceptionListenerTest extends TestCase +class ErrorListenerTest extends TestCase { public function testOnConsoleError() { - $exception = new \RuntimeException('An error occurred'); + $error = new \TypeError('An error occurred'); $logger = $this->getLogger(); $logger ->expects($this->once()) ->method('error') - ->with('Error thrown while running command "{command}". Message: "{message}"', array('error' => $exception, 'command' => 'test:run --foo=baz buzz', 'message' => 'An error occurred')) + ->with('Error thrown while running command "{command}". Message: "{message}"', array('error' => $error, 'command' => 'test:run --foo=baz buzz', 'message' => 'An error occurred')) ; - $listener = new ExceptionListener($logger); - $listener->onConsoleError($this->getConsoleErrorEvent($exception, new ArgvInput(array('console.php', 'test:run', '--foo=baz', 'buzz')), 1, new Command('test:run'))); + $listener = new ErrorListener($logger); + $listener->onConsoleError(new ConsoleErrorEvent(new ArgvInput(array('console.php', 'test:run', '--foo=baz', 'buzz')), $this->getOutput(), $error, new Command('test:run'))); } public function testOnConsoleErrorWithNoCommandAndNoInputString() { - $exception = new \RuntimeException('An error occurred'); + $error = new \RuntimeException('An error occurred'); $logger = $this->getLogger(); $logger ->expects($this->once()) ->method('error') - ->with('An error occurred while using the console. Message: "{message}"', array('error' => $exception, 'message' => 'An error occurred')) + ->with('An error occurred while using the console. Message: "{message}"', array('error' => $error, 'message' => 'An error occurred')) ; - $listener = new ExceptionListener($logger); - $listener->onConsoleError($this->getConsoleErrorEvent($exception, new NonStringInput(), 1)); + $listener = new ErrorListener($logger); + $listener->onConsoleError(new ConsoleErrorEvent(new NonStringInput(), $this->getOutput(), $error)); } public function testOnConsoleTerminateForNonZeroExitCodeWritesToLog() @@ -65,7 +65,7 @@ public function testOnConsoleTerminateForNonZeroExitCodeWritesToLog() ->with('Command "{command}" exited with code "{code}"', array('command' => 'test:run', 'code' => 255)) ; - $listener = new ExceptionListener($logger); + $listener = new ErrorListener($logger); $listener->onConsoleTerminate($this->getConsoleTerminateEvent(new ArgvInput(array('console.php', 'test:run')), 255)); } @@ -77,7 +77,7 @@ public function testOnConsoleTerminateForZeroExitCodeDoesNotWriteToLog() ->method('error') ; - $listener = new ExceptionListener($logger); + $listener = new ErrorListener($logger); $listener->onConsoleTerminate($this->getConsoleTerminateEvent(new ArgvInput(array('console.php', 'test:run')), 0)); } @@ -88,7 +88,7 @@ public function testGetSubscribedEvents() 'console.error' => array('onConsoleError', -128), 'console.terminate' => array('onConsoleTerminate', -128), ), - ExceptionListener::getSubscribedEvents() + ErrorListener::getSubscribedEvents() ); } @@ -101,7 +101,7 @@ public function testAllKindsOfInputCanBeLogged() ->with('Command "{command}" exited with code "{code}"', array('command' => 'test:run --foo=bar', 'code' => 255)) ; - $listener = new ExceptionListener($logger); + $listener = new ErrorListener($logger); $listener->onConsoleTerminate($this->getConsoleTerminateEvent(new ArgvInput(array('console.php', 'test:run', '--foo=bar')), 255)); $listener->onConsoleTerminate($this->getConsoleTerminateEvent(new ArrayInput(array('name' => 'test:run', '--foo' => 'bar')), 255)); $listener->onConsoleTerminate($this->getConsoleTerminateEvent(new StringInput('test:run --foo=bar'), 255)); @@ -116,7 +116,7 @@ public function testCommandNameIsDisplayedForNonStringableInput() ->with('Command "{command}" exited with code "{code}"', array('command' => 'test:run', 'code' => 255)) ; - $listener = new ExceptionListener($logger); + $listener = new ErrorListener($logger); $listener->onConsoleTerminate($this->getConsoleTerminateEvent($this->getMockBuilder(InputInterface::class)->getMock(), 255)); } @@ -125,11 +125,6 @@ private function getLogger() return $this->getMockForAbstractClass(LoggerInterface::class); } - private function getConsoleErrorEvent(\Exception $exception, InputInterface $input, $exitCode, Command $command = null) - { - return new ConsoleErrorEvent($input, $this->getOutput(), $exception, $exitCode, $command); - } - private function getConsoleTerminateEvent(InputInterface $input, $exitCode) { return new ConsoleTerminateEvent(new Command('test:run'), $input, $this->getOutput(), $exitCode);