It's surely debatable but I would use $catchErrors instead of $catchThrowables since exceptions are throwable.

It's surely debatable but I would use $catchErrors instead of $catchThrowables since exceptions are throwable.

I asked myself about that. But it might be misleading for PHP 5 users (actual php 5 errors won't be handled by the application). Also throwables does not mean a PHP error, but any other class implementing \Throwable.

Also throwables does not mean a PHP error, but any other class implementing \Throwable.

From the PHP manual:

PHP classes cannot implement the Throwable interface directly

Which class being neither a subclass of \Error nor \Exception could be throwable?

PHP classes cannot implement the Throwable interface directly

You're right. Forget about this argument.

Still it might be misleading for PHP 5 users. Unless we register renderThrowable/Error as an error handler if catchThrowables/catchErrors is true ?

How bad would it be to handle errors by using the Debug component as in 1cf2545 ? Calling Debug::enable outside from a bootstrapping file looks hazardous to me, but what could be proposed instead, or how to make it safer/reliable? Should we consider that the end-user opted-in by using running this application with catchErrors and accept it may have a wider effect?
Just asking, but we should probably not do anything for PHP 5 after all and stick to PHP 7 errors handling...

I would not rename renderException(). The whole Throwable/Error/Exception hierarchy is there in PHP for BC. There is no need to expose that to our users. At the end of the day, they are dealing with exceptions.

Fair enough. Last chance for this then is to create a special exception wrapping an \Error, allowing to pass it to renderException.

There is no more BC break nor deprecation here AFAIK. So the label can be removed :)

The typehint will however change in 4.0 without BC layer between, as it was done in 3.0 (#13756). Or we can keep the internal ErrorException and keep the typehint on \Exception.

I've just realized the debug component was already required by the console component, thus I don't need the internal Symfony\Component\Console\Exception\ErrorException but can directly use FatalThrowableError (apparently symfony/debug is only required for this class).

I've also removed the notice in UPGRADE-4.0.md, unless we indeed decide to handle \Throwable in Application::renderException().

Rethinking about this, I think the $catchErrors flag should be reverted and errors should always be converted and handled as any exception.

Still doesn't look sensible to me to loose informations brung by the Symfony\Component\Debug\ErrorHandler and output by the console thanks to the Symfony\Component\HttpKernel\EventListener\DebugHandlersListener (as mentioned in #20808 (comment)).

If I apply the following patch:

diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php
index fd12aa0..6175332 100644
--- a/src/Symfony/Component/Console/Application.php
+++ b/src/Symfony/Component/Console/Application.php
@@ -36,6 +36,7 @@ use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\Console\Event\ConsoleTerminateEvent;
 use Symfony\Component\Console\Exception\CommandNotFoundException;
 use Symfony\Component\Console\Exception\LogicException;
+use Symfony\Component\Debug\ErrorHandler;
 use Symfony\Component\Debug\Exception\FatalThrowableError;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -62,7 +63,6 @@ class Application
     private $name;
     private $version;
     private $catchExceptions = true;
-    private $catchErrors = false;
     private $autoExit = true;
     private $definition;
     private $helperSet;
@@ -124,9 +124,10 @@ class Application
                 throw $e;
             }
         } catch (\Error $e) {
-            if (!$this->catchErrors) {
+            if (!$this->catchExceptions) {
                 throw $e;
             }
+
             $e = new FatalThrowableError($e);
         }
 
@@ -278,22 +279,6 @@ class Application
     }
 
     /**
-     * @return bool Whether errors are caught or not during commands execution
-     */
-    public function areErrorsCaught()
-    {
-        return $this->catchErrors;
-    }
-
-    /**
-     * @param bool $boolean Whether to catch errors or not during commands execution
-     */
-    public function setCatchErrors($boolean)
-    {
-        $this->catchErrors = (bool) $boolean;
-    }
-
-    /**
      * Gets whether to automatically exit after a command execution or not.
      *
      * @return bool Whether to automatically exit after a command execution or not

the output changes from:

to

If I try to detect the Symfony ErrorHandler was registered using this patch:

diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php
index fd12aa0..ddc81f1 100644
--- a/src/Symfony/Component/Console/Application.php
+++ b/src/Symfony/Component/Console/Application.php
@@ -36,6 +36,7 @@ use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\Console\Event\ConsoleTerminateEvent;
 use Symfony\Component\Console\Exception\CommandNotFoundException;
 use Symfony\Component\Console\Exception\LogicException;
+use Symfony\Component\Debug\ErrorHandler;
 use Symfony\Component\Debug\Exception\FatalThrowableError;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
@@ -62,7 +63,6 @@ class Application
     private $name;
     private $version;
     private $catchExceptions = true;
-    private $catchErrors = false;
     private $autoExit = true;
     private $definition;
     private $helperSet;
@@ -124,9 +124,18 @@ class Application
                 throw $e;
             }
         } catch (\Error $e) {
-            if (!$this->catchErrors) {
+            if (!$this->catchExceptions) {
+                throw $e;
+            }
+
+            $handler = set_exception_handler('var_dump');
+            $handler = is_array($handler) ? $handler[0] : null;
+            restore_exception_handler();
+            // Symfony Debug error handler was registered, let it handle the error:
+            if ($handler instanceof ErrorHandler) {
                 throw $e;
             }
+
             $e = new FatalThrowableError($e);
         }
 
@@ -278,22 +287,6 @@ class Application
     }
 
     /**
-     * @return bool Whether errors are caught or not during commands execution
-     */
-    public function areErrorsCaught()
-    {
-        return $this->catchErrors;
-    }
-
-    /**
-     * @param bool $boolean Whether to catch errors or not during commands execution
-     */
-    public function setCatchErrors($boolean)
-    {
-        $this->catchErrors = (bool) $boolean;
-    }
-
-    /**
      * Gets whether to automatically exit after a command execution or not.
      *
      * @return bool Whether to automatically exit after a command execution or not

problem is solved using the fullstack. But that's wrong, as it only tells the ErrorHandler was set, like in any app using Debug::enable(). It doesn't ensure the following has been registered as exception handler (this is what is done by the DebugHandlersListener):

function ($e) use ($app, $output) {
    $app->renderException($e, $output);
};

So any standalone console app using the Debug component would get the raw output instead of being nicely handled by the console component.

Maybe we could try looking if the DebugHandlersListener is registered, but doing such a thing in the Application class looks inappropriate.

Also we should remember that despite the issue I'm trying to describe is about Symfony's ErrorHandler, someone using its own way to output errors to console would encounter the same issues if we're suddenly handling errors in the Application class.

So, yes, I can revert changes to handle errors and exceptions exactly the same way, without this flag (PATCH 1). I just want to be sure everyone is aware of this. To me it's a stumbling block. 😕

At least, the new flag is opt-in and won't cause more troubles than adding 1 line to enable errors catching in a standalone console app.

However, if anyone has another clean and BC solution regarding this, I'll be glad to give it a try. :)

Is it still relevant? We fixed quite a few bugs related to that recently.

I need to check, but last time I did, it was still needed to me as trying to explain in my last comment. 😕

I didn't follow much changes lately. Which commits should have enhanced the situation regarding this?

Since #18140 was merged ( ping @wouterj ), this one does not make sense anymore as is (or should revert some changes of #18140). But as explained, catching errors indistinctly from exceptions could be considered as a BC break, as error handlers are not used anymore. Using the same catchExceptions flag for both exceptions & errors prevents us from having a clean path to ensure the BC.

So you can close this as soon as the BC break is acknowledged but considered acceptable to you. But this is also a DX regression in the case of the FrameworkBundle, which used to register a handler & enhance the messages, but doesn't anymore now. :/

Could you confirm #22261 fixes the BC break? I'm not able to add a relevant test for it

I think we can close this one, I'm going to revisit the issues spotted here as introduced by #18140 separately.