From 46b6b42b7ba884d140a3f8457752e802b039db89 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Wed, 9 Aug 2017 18:38:20 +0200 Subject: [PATCH] [FrameworkBundle] Catch Fatal errors in commands registration --- .../FrameworkBundle/Console/Application.php | 51 ++++++++++++++++++- .../Tests/Console/ApplicationTest.php | 43 ++++++++++++++++ src/Symfony/Component/Console/Application.php | 15 ++++-- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php index b1c893ad05..09c61933ee 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php @@ -11,6 +11,9 @@ namespace Symfony\Bundle\FrameworkBundle\Console; +use Symfony\Component\Console\Output\ConsoleOutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; +use Symfony\Component\Debug\Exception\FatalThrowableError; use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\Console\Application as BaseApplication; use Symfony\Component\Console\Command\Command; @@ -30,6 +33,7 @@ class Application extends BaseApplication { private $kernel; private $commandsRegistered = false; + private $registrationErrors = array(); /** * Constructor. @@ -70,9 +74,25 @@ class Application extends BaseApplication $this->setDispatcher($this->kernel->getContainer()->get('event_dispatcher')); + if ($this->registrationErrors) { + $this->renderRegistrationErrors($input, $output); + } + return parent::doRun($input, $output); } + /** + * {@inheritdoc} + */ + protected function doRunCommand(Command $command, InputInterface $input, OutputInterface $output) + { + if ($this->registrationErrors) { + $this->renderRegistrationErrors($input, $output); + } + + return parent::doRunCommand($command, $input, $output); + } + /** * {@inheritdoc} */ @@ -138,7 +158,13 @@ class Application extends BaseApplication foreach ($this->kernel->getBundles() as $bundle) { if ($bundle instanceof Bundle) { - $bundle->registerCommands($this); + try { + $bundle->registerCommands($this); + } catch (\Exception $e) { + $this->registrationErrors[] = $e; + } catch (\Throwable $e) { + $this->registrationErrors[] = new FatalThrowableError($e); + } } } @@ -149,9 +175,30 @@ class Application extends BaseApplication if ($container->hasParameter('console.command.ids')) { foreach ($container->getParameter('console.command.ids') as $id) { if (false !== $id) { - $this->add($container->get($id)); + try { + $this->add($container->get($id)); + } catch (\Exception $e) { + $this->registrationErrors[] = $e; + } catch (\Throwable $e) { + $this->registrationErrors[] = new FatalThrowableError($e); + } } } } } + + private function renderRegistrationErrors(InputInterface $input, OutputInterface $output) + { + if ($output instanceof ConsoleOutputInterface) { + $output = $output->getErrorOutput(); + } + + (new SymfonyStyle($input, $output))->warning('Some commands could not be registered.'); + + foreach ($this->registrationErrors as $error) { + $this->doRenderException($error, $output); + } + + $this->registrationErrors = array(); + } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php index 25511142c9..ce7ebad1f1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php @@ -15,8 +15,13 @@ use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Bundle\FrameworkBundle\Tests\TestCase; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\ArrayInput; +use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\NullOutput; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\ApplicationTester; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\HttpKernel\KernelInterface; class ApplicationTest extends TestCase { @@ -130,6 +135,36 @@ class ApplicationTest extends TestCase $this->assertSame($newCommand, $application->get('example')); } + public function testRunOnlyWarnsOnUnregistrableCommand() + { + $container = new ContainerBuilder(); + $container->register('event_dispatcher', EventDispatcher::class); + $container->register(ThrowingCommand::class, ThrowingCommand::class); + $container->setParameter('console.command.ids', array(ThrowingCommand::class => ThrowingCommand::class)); + + $kernel = $this->getMockBuilder(KernelInterface::class)->getMock(); + $kernel + ->method('getBundles') + ->willReturn(array($this->createBundleMock( + array((new Command('fine'))->setCode(function (InputInterface $input, OutputInterface $output) { $output->write('fine'); })) + ))); + $kernel + ->method('getContainer') + ->willReturn($container); + + $application = new Application($kernel); + $application->setAutoExit(false); + + $tester = new ApplicationTester($application); + $tester->run(array('command' => 'fine')); + $output = $tester->getDisplay(); + + $this->assertSame(0, $tester->getStatusCode()); + $this->assertContains('Some commands could not be registered.', $output); + $this->assertContains('throwing', $output); + $this->assertContains('fine', $output); + } + private function getKernel(array $bundles, $useDispatcher = false) { $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerInterface')->getMock(); @@ -189,3 +224,11 @@ class ApplicationTest extends TestCase return $bundle; } } + +class ThrowingCommand extends Command +{ + public function __construct() + { + throw new \Exception('throwing'); + } +} diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index 627a2b1db2..b9669789ea 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -706,6 +706,16 @@ class Application { $output->writeln('', OutputInterface::VERBOSITY_QUIET); + $this->doRenderException($e, $output); + + if (null !== $this->runningCommand) { + $output->writeln(sprintf('%s', sprintf($this->runningCommand->getSynopsis(), $this->getName())), OutputInterface::VERBOSITY_QUIET); + $output->writeln('', OutputInterface::VERBOSITY_QUIET); + } + } + + protected function doRenderException(\Exception $e, OutputInterface $output) + { do { $title = sprintf( ' [%s%s] ', @@ -767,11 +777,6 @@ class Application $output->writeln('', OutputInterface::VERBOSITY_QUIET); } } while ($e = $e->getPrevious()); - - if (null !== $this->runningCommand) { - $output->writeln(sprintf('%s', sprintf($this->runningCommand->getSynopsis(), $this->getName())), OutputInterface::VERBOSITY_QUIET); - $output->writeln('', OutputInterface::VERBOSITY_QUIET); - } } /**