feature #23836 [FrameworkBundle] Catch Fatal errors in commands registration (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Catch Fatal errors in commands registration

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#23610
| License       | MIT
| Doc PR        | n/a

#### before

![before](https://image.prntscr.com/image/QDmEVSPiQY6VCY7PoNAIbg.png)

#### after

![after](https://image.prntscr.com/image/B2EuZtzwTfiVS6F3kTAa4Q.png)

Trying to run the failing command itself

![failing](https://image.prntscr.com/image/7RgdtCVyQXyVit8TPkh98A.png)

Exceptions/errors thrown in `registerCommands()` (bundles/non-lazy command services registration) are caught and displayed as a warning, allowing to run other valid commands.

Commits
-------

46b6b42b7b [FrameworkBundle] Catch Fatal errors in commands registration

Author: fabpot

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 @@ public function doRun(InputInterface $input, OutputInterface $output)
 
         $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 @@ protected function registerCommands()
 
         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 @@ protected function registerCommands()
         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();
+    }
 }

Tests/Console/ApplicationTest.php

@@ -15,8 +15,13 @@
 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 @@ public function testBundleCommandCanOverriddeAPreExistingCommandWithTheSameName(
         $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 @@ private function createBundleMock(array $commands)
         return $bundle;
     }
 }
+
+class ThrowingCommand extends Command
+{
+    public function __construct()
+    {
+        throw new \Exception('throwing');
+    }
+}