chore: Always catch errors when calling console

[llupa]

Q	A
License	MIT
Doc issue/PR	#53946

Pinging @wouterj @greg0ire

This is my first time doing "recipe" work and I am not sure if this is the correct place to open this PR. Let me know for any change needed!

In short: when executing console commands in production mode/env commands have to fail with exit 1, and not render the \Error and exit 0.

[github-actions]

Thanks for the PR 😍

How to test these changes in your application

1. Define the SYMFONY_ENDPOINT environment variable:

   # On Unix-like (BSD, Linux and macOS)
   export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1296/index.json
   # On Windows
   SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1296/index.json

2. Install the package(s) related to this recipe:

   composer req 'symfony/flex:^1.16'
   composer req 'symfony/console:^6.4'

3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

   # On Unix-like (BSD, Linux and macOS)
   unset SYMFONY_ENDPOINT
   # On Windows
   SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/console

3.3 vs 4.2

diff --git a/symfony/console/3.3/config/bootstrap.php b/symfony/console/4.2/config/bootstrap.php
index 2a47186..55560fb 100644
--- a/symfony/console/3.3/config/bootstrap.php
+++ b/symfony/console/4.2/config/bootstrap.php
@@ -13,38 +13,8 @@ if (!class_exists(Dotenv::class)) {
 if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
     (new Dotenv(false))->populate($env);
 } else {
-    $path = dirname(__DIR__).'/.env';
-    $dotenv = new Dotenv(false);
-
     // load all the .env files
-    if (method_exists($dotenv, 'loadEnv')) {
-        $dotenv->loadEnv($path);
-    } else {
-        // fallback code in case your Dotenv component is not 4.2 or higher (when loadEnv() was added)
-
-        if (file_exists($path) || !file_exists($p = "$path.dist")) {
-            $dotenv->load($path);
-        } else {
-            $dotenv->load($p);
-        }
-
-        if (null === $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) {
-            $dotenv->populate(array('APP_ENV' => $env = 'dev'));
-        }
-
-        if ('test' !== $env && file_exists($p = "$path.local")) {
-            $dotenv->load($p);
-            $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env;
-        }
-
-        if (file_exists($p = "$path.$env")) {
-            $dotenv->load($p);
-        }
-
-        if (file_exists($p = "$path.$env.local")) {
-            $dotenv->load($p);
-        }
-    }
+    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
 }
 
 $_SERVER += $_ENV;

4.2 vs 4.4

diff --git a/symfony/console/4.2/bin/console b/symfony/console/4.4/bin/console
index 5cef879..5de0e1c 100755
--- a/symfony/console/4.2/bin/console
+++ b/symfony/console/4.4/bin/console
@@ -4,7 +4,7 @@
 use App\Kernel;
 use Symfony\Bundle\FrameworkBundle\Console\Application;
 use Symfony\Component\Console\Input\ArgvInput;
-use Symfony\Component\Debug\Debug;
+use Symfony\Component\ErrorHandler\Debug;
 
 if (!in_array(PHP_SAPI, ['cli', 'phpdbg', 'embed'], true)) {
     echo 'Warning: The console should be invoked via the CLI version of PHP, not the '.PHP_SAPI.' SAPI'.PHP_EOL;

4.4 vs 5.1

diff --git a/symfony/console/4.4/bin/console b/symfony/console/5.1/bin/console
index 5de0e1c..8fe9d49 100755
--- a/symfony/console/4.4/bin/console
+++ b/symfony/console/5.1/bin/console
@@ -4,6 +4,7 @@
 use App\Kernel;
 use Symfony\Bundle\FrameworkBundle\Console\Application;
 use Symfony\Component\Console\Input\ArgvInput;
+use Symfony\Component\Dotenv\Dotenv;
 use Symfony\Component\ErrorHandler\Debug;
 
 if (!in_array(PHP_SAPI, ['cli', 'phpdbg', 'embed'], true)) {
@@ -14,8 +15,8 @@ set_time_limit(0);
 
 require dirname(__DIR__).'/vendor/autoload.php';
 
-if (!class_exists(Application::class)) {
-    throw new LogicException('You need to add "symfony/framework-bundle" as a Composer dependency.');
+if (!class_exists(Application::class) || !class_exists(Dotenv::class)) {
+    throw new LogicException('You need to add "symfony/framework-bundle" and "symfony/dotenv" as Composer dependencies.');
 }
 
 $input = new ArgvInput();
@@ -27,7 +28,7 @@ if ($input->hasParameterOption('--no-debug', true)) {
     putenv('APP_DEBUG='.$_SERVER['APP_DEBUG'] = $_ENV['APP_DEBUG'] = '0');
 }
 
-require dirname(__DIR__).'/config/bootstrap.php';
+(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
 
 if ($_SERVER['APP_DEBUG']) {
     umask(0000);
diff --git a/symfony/console/4.4/config/bootstrap.php b/symfony/console/4.4/config/bootstrap.php
deleted file mode 100644
index 55560fb..0000000
--- a/symfony/console/4.4/config/bootstrap.php
+++ /dev/null
@@ -1,23 +0,0 @@
-<?php
-
-use Symfony\Component\Dotenv\Dotenv;
-
-require dirname(__DIR__).'/vendor/autoload.php';
-
-if (!class_exists(Dotenv::class)) {
-    throw new LogicException('Please run "composer require symfony/dotenv" to load the ".env" files configuring the application.');
-}
-
-// Load cached env vars if the .env.local.php file exists
-// Run "composer dump-env prod" to create it (requires symfony/flex >=1.2)
-if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
-    (new Dotenv(false))->populate($env);
-} else {
-    // load all the .env files
-    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
-}
-
-$_SERVER += $_ENV;
-$_SERVER['APP_ENV'] = $_ENV['APP_ENV'] = ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) ?: 'dev';
-$_SERVER['APP_DEBUG'] = $_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? 'prod' !== $_SERVER['APP_ENV'];
-$_SERVER['APP_DEBUG'] = $_ENV['APP_DEBUG'] = (int) $_SERVER['APP_DEBUG'] || filter_var($_SERVER['APP_DEBUG'], FILTER_VALIDATE_BOOLEAN) ? '1' : '0';
diff --git a/symfony/console/4.4/manifest.json b/symfony/console/5.1/manifest.json
index c00421d..b0e9e1d 100644
--- a/symfony/console/4.4/manifest.json
+++ b/symfony/console/5.1/manifest.json
@@ -1,7 +1,6 @@
 {
     "copy-from-recipe": {
-        "bin/": "%BIN_DIR%/",
-        "config/": "%CONFIG_DIR%/"
+        "bin/": "%BIN_DIR%/"
     },
     "aliases": ["cli"]
 }

5.1 vs 5.3

diff --git a/symfony/console/5.1/bin/console b/symfony/console/5.3/bin/console
index 8fe9d49..c933dc5 100755
--- a/symfony/console/5.1/bin/console
+++ b/symfony/console/5.3/bin/console
@@ -3,41 +3,15 @@
 
 use App\Kernel;
 use Symfony\Bundle\FrameworkBundle\Console\Application;
-use Symfony\Component\Console\Input\ArgvInput;
-use Symfony\Component\Dotenv\Dotenv;
-use Symfony\Component\ErrorHandler\Debug;
 
-if (!in_array(PHP_SAPI, ['cli', 'phpdbg', 'embed'], true)) {
-    echo 'Warning: The console should be invoked via the CLI version of PHP, not the '.PHP_SAPI.' SAPI'.PHP_EOL;
+if (!is_file(dirname(__DIR__).'/vendor/autoload_runtime.php')) {
+    throw new LogicException('Symfony Runtime is missing. Try running "composer require symfony/runtime".');
 }
 
-set_time_limit(0);
+require_once dirname(__DIR__).'/vendor/autoload_runtime.php';
 
-require dirname(__DIR__).'/vendor/autoload.php';
+return function (array $context) {
+    $kernel = new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
 
-if (!class_exists(Application::class) || !class_exists(Dotenv::class)) {
-    throw new LogicException('You need to add "symfony/framework-bundle" and "symfony/dotenv" as Composer dependencies.');
-}
-
-$input = new ArgvInput();
-if (null !== $env = $input->getParameterOption(['--env', '-e'], null, true)) {
-    putenv('APP_ENV='.$_SERVER['APP_ENV'] = $_ENV['APP_ENV'] = $env);
-}
-
-if ($input->hasParameterOption('--no-debug', true)) {
-    putenv('APP_DEBUG='.$_SERVER['APP_DEBUG'] = $_ENV['APP_DEBUG'] = '0');
-}
-
-(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
-
-if ($_SERVER['APP_DEBUG']) {
-    umask(0000);
-
-    if (class_exists(Debug::class)) {
-        Debug::enable();
-    }
-}
-
-$kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
-$application = new Application($kernel);
-$application->run($input);
+    return new Application($kernel);
+};

5.3 vs 6.4

diff --git a/symfony/console/5.3/bin/console b/symfony/console/6.4/bin/console
index c933dc5..5573910 100755
--- a/symfony/console/5.3/bin/console
+++ b/symfony/console/6.4/bin/console
@@ -13,5 +13,8 @@ require_once dirname(__DIR__).'/vendor/autoload_runtime.php';
 return function (array $context) {
     $kernel = new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
 
-    return new Application($kernel);
+    $app = new Application($kernel);
+    $app->setCatchErrors(true);
+
+    return $app;
 };

[nicolas-grekas]

This would be a work around. Let's fix the root issue instead, see symfony/symfony#54189

[wouterj]

@nicolas-grekas I think aside of this issue that triggered this PR, we might want to still do this.

The console app now defaults to catchExceptions: true and catchErrors: false. I think it makes sense to make both true in Symfony 8, meaning the console terminate event is dispatched for both exceptions and errors.
Thus we can do the traditional BC cycle here: deprecate not setting catchErrors, change recipe to always use new default (this PR), and in 8.0, swap default around, undeprecate not setting catchErrors and update recipe to not override the default.
What do you think?

(btw, thanks for providing a patch in the error handler!)

[llupa]

Not as detailed as Wouter's comment, but I also will have to set it to true for my projects because I cannot really think of a use case (DX) where I want Exceptions to be caught and rendered one way, but Errors not caught (probably will be after your patch) but rendered a different way.

Maybe it boils down to personal preference, but it is odd that it works differently for one from the other.

[nicolas-grekas]

I think we should minimize those flip-flap with deprecations so I'd better not change anything personally.
Also the current logic is perfectly fine to me. Error are split from Exception because they're not the same beast.