Symfony compliant error handling

.phpstan-baseline.php

@@ -1735,12 +1735,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/src/Fieldblacklist.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Property GLPI\\:\\:\\$error_handler is unused\\.$#',
-	'identifier' => 'property.unused',
-	'count' => 1,
-	'path' => __DIR__ . '/src/GLPI.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Method GLPIKey\\:\\:keyExists\\(\\) should return string but returns bool\\.$#',
 	'identifier' => 'return.type',
@@ -2065,24 +2059,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/src/Glpi/Api/HL/Search.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Instanceof between Psr\\\\Log\\\\LoggerInterface and Psr\\\\Log\\\\LoggerInterface will always evaluate to true\\.$#',
-	'identifier' => 'instanceof.alwaysTrue',
-	'count' => 1,
-	'path' => __DIR__ . '/src/Glpi/Application/ErrorHandler.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Property Glpi\\\\Application\\\\ErrorHandler\\:\\:\\$exit_code is never read, only written\\.$#',
-	'identifier' => 'property.onlyWritten',
-	'count' => 1,
-	'path' => __DIR__ . '/src/Glpi/Application/ErrorHandler.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Strict comparison using \\!\\=\\= between null and \'comment\'\\|\'error\' will always evaluate to true\\.$#',
-	'identifier' => 'notIdentical.alwaysTrue',
-	'count' => 1,
-	'path' => __DIR__ . '/src/Glpi/Application/ErrorHandler.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Strict comparison using \\=\\=\\= between null and 2 will always evaluate to false\\.$#',
 	'identifier' => 'identical.alwaysFalse',

CHANGELOG.md

@@ -166,7 +166,6 @@ The present file will list all changes made to the project; according to the
 - `Glpi\Dashboard\Filters\AbstractFilter::field()` method has been made protected.
 - Usage of `CommonITILValidation::dropdownValidator()` with the `name` and `users_id_validate` options are no longer supported. Use `prefix` and `itemtype_target`/`items_id_target` respectively instead.
 - The `helper` property of form fields will not support anymore the presence of HTML code.
-- `Glpi\Application\ErrorHandler` constructor visibility has been changed to private.
 - `GLPI::initErrorHandler()` does not return any value anymore.
 - The `inc/autoload.function.php`, `inc/based_config.php`, `inc/config.php`, `inc/db.function.php` and `inc/define.php` files have been removed and the `inc/includes.php` file has been almost emptied.
   The corresponding global functions, constants and variables are now loaded and initialized automatically and the corresponding GLPI boostraping logic is now executed automatically.
@@ -397,11 +396,11 @@ The present file will list all changes made to the project; according to the
 - `FieldUnicity::showDebug()`
 - `GLPI::getErrorHandler()`
 - `GLPI::getLogLevel()`
+- `GLPI::initErrorHandler()`
+- `GLPI::initLogger()`
 - `Glpi\Api\API::showDebug()`
 - `Glpi\Api\API::returnSanitizedContent()`
-- `Glpi\Application\ErrorHandler::handleSqlError()`
-- `Glpi\Application\ErrorHandler::handleSqlWarnings()`
-- `Glpi\Application\ErrorHandler::handleTwigError()`
+- `Glpi\Application\ErrorHandler` class
 - `Glpi\Console\Command\ForceNoPluginsOptionCommandInterface` class
 - `Glpi\Dashboard\Filter::dates()`
 - `Glpi\Dashboard\Filter::dates_mod()`

ajax/dashboard.php

@@ -33,8 +33,8 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
 use Glpi\Dashboard\Grid;
+use Glpi\Error\ErrorHandler;
 use Glpi\Exception\Http\AccessDeniedHttpException;
 
 if (!isset($_REQUEST["action"])) {
@@ -208,8 +208,7 @@
                 $result[$card['card_id']] = $grid->getCardHtml($card['card_id'], array_merge($request_data, $card));
             } catch (\Throwable $e) {
                 // Send exception to logger without actually exiting.
-                // Use quiet mode to not break JSON result.
-                ErrorHandler::getInstance()->handleException($e, true);
+                ErrorHandler::logCaughtException($e);
             }
         }
         \Glpi\Debug\Profiler::getInstance()->stop('Get cards HTML');

ajax/debug.php

@@ -33,7 +33,6 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
 use Glpi\Exception\Http\AccessDeniedHttpException;
 use Glpi\Exception\Http\BadRequestHttpException;
 use Glpi\Exception\Http\NotFoundHttpException;
@@ -99,8 +98,6 @@
             echo '[]';
             return;
         }
-        // In some cases, a class that isn't a proper itemtype may show in the selection box and this would trigger a SQL error that cannot be caught.
-        ErrorHandler::getInstance()->disableOutput();
         try {
             /** @var CommonGLPI $item */
             $item = new $_GET['itemtype']();

ajax/fileupload.php

@@ -37,9 +37,4 @@
  * @since 9.2
  */
 
-use Glpi\Application\ErrorHandler;
-
-// Ensure warnings will not break ajax output.
-ErrorHandler::getInstance()->disableOutput();
-
 GLPIUploadHandler::uploadFiles($_POST);

dependency_injection/services.php

@@ -35,7 +35,7 @@
 namespace Symfony\Component\DependencyInjection\Loader\Configurator;
 
 use Glpi\DependencyInjection\PublicService;
-use Glpi\Log\LegacyGlobalLogger;
+use Glpi\Error\ErrorHandler;
 
 return static function (ContainerConfigurator $container): void {
     $projectDir = dirname(__DIR__);
@@ -47,6 +47,9 @@
     $parameters->set('env(APP_SECRET_FILE)', $projectDir . '/config/glpicrypt.key');
     $parameters->set('kernel.secret', env('default:glpi.default_secret:file:APP_SECRET_FILE'));
 
+    // Prevent low level errors (e.g. warning) to be converted to exception in dev environment
+    $parameters->set('debug.error_handler.throw_at', ErrorHandler::FATAL_ERRORS);
+
     $services = $services
         ->defaults()
             ->autowire()
@@ -64,9 +67,7 @@
         $projectDir . '/src/Glpi/Form/ConditionalVisiblity/*Manager.php'
     );
 
-    /**
-     * Override Symfony's logger.
-     * @see \Symfony\Component\HttpKernel\DependencyInjection\LoggerPass
-     */
-    $services->set('logger', LegacyGlobalLogger::class);
+    // Prevent Symfony to register its own default logger.
+    // @see \Symfony\Component\HttpKernel\DependencyInjection\LoggerPass
+    $services->set('logger')->synthetic();
 };

front/css.php

@@ -33,7 +33,6 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
 use Glpi\UI\ThemeManager;
 
 // Main CSS compilation requires about 70MB of memory.
@@ -43,9 +42,6 @@
     ini_set('memory_limit', sprintf('%dM', $max_memory));
 }
 
-// Ensure warnings will not break CSS output.
-ErrorHandler::getInstance()->disableOutput();
-
 // If a custom theme is requested, we need to get the real path of the theme
 if (isset($_GET['file']) && isset($_GET['is_custom_theme']) && $_GET['is_custom_theme']) {
     $theme = ThemeManager::getInstance()->getTheme($_GET['file']);

front/locale.php

@@ -33,7 +33,7 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
+use Glpi\Error\ErrorHandler;
 
 /**
  * @var array $CFG_GLPI
@@ -78,7 +78,7 @@
     $messages = $TRANSLATE->getAllMessages($_GET['domain']);
 } catch (\Throwable $e) {
     // Error may happen when overrided translation files does not use same plural rules as GLPI.
-    ErrorHandler::getInstance()->handleException($e, true);
+    ErrorHandler::logCaughtException($e);
 }
 if (!($messages instanceof \Laminas\I18n\Translator\TextDomain)) {
    // No TextDomain found means that there is no translations for given domain.

front/notificationmailingsetting.form.php

@@ -33,7 +33,7 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
+use Glpi\Error\ErrorHandler;
 use Glpi\Event;
 use Glpi\Mail\SMTP\OauthConfig;
 
@@ -58,7 +58,7 @@
                 $_SESSION['smtp_oauth2_state'] = $provider->getState();
                 Html::redirect($auth_url);
             } catch (\Throwable $e) {
-                ErrorHandler::getInstance()->handleException($e, true);
+                ErrorHandler::logCaughtException($e);
                 Session::addMessageAfterRedirect(
                     htmlescape(sprintf(_x('oauth', 'Authorization failed with error: %s'), $e->getMessage())),
                     false,

front/palette_preview.php

@@ -33,12 +33,8 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
 use Glpi\UI\ThemeManager;
 
-// Ensure warnings will not break image output.
-ErrorHandler::getInstance()->disableOutput();
-
 $theme = ThemeManager::getInstance()->getTheme($_GET['key']);
 $preview = $theme !== null ? $theme->getPreviewPath(false) : null;
 

install/install.php

@@ -167,7 +167,6 @@ function step2($update)
 function step3($host, $user, $password, $update)
 {
 
-    error_reporting(16);
     mysqli_report(MYSQLI_REPORT_OFF);
 
    //Check if the port is in url
@@ -482,13 +481,6 @@ function update1($dbname)
     }
 }
 
-
-
-//------------Start of install script---------------------------
-
-error_reporting(0); // we want to check system before affraid the user.
-
-
 /**
  * @since 0.84.2
  **/
@@ -509,6 +501,10 @@ function checkConfigFile()
     Html::redirect($CFG_GLPI['root_doc'] . "/index.php");
 }
 
+
+//------------Start of install script---------------------------
+
+
 if (!isset($_SESSION['can_process_install']) || !isset($_POST["install"])) {
     $_SESSION = [];
 

phpunit/bootstrap.php

@@ -33,15 +33,11 @@
  * ---------------------------------------------------------------------
  */
 
-use Glpi\Application\ErrorHandler;
 use Glpi\Cache\CacheManager;
 use Glpi\Cache\SimpleCache;
 use Glpi\Kernel\Kernel;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 
-ini_set('display_errors', 'On'); // Ensure errors happening during test suite bootstrapping are always displayed
-error_reporting(E_ALL);
-
 define('GLPI_URI', getenv('GLPI_URI') ?: 'http://localhost:80');
 define('GLPI_STRICT_DEPRECATED', true); //enable strict depreciations
 
@@ -95,8 +91,3 @@
 $tu_oauth_client->getFromDBByCrit(['name' => 'Test OAuth Client']);
 define('TU_OAUTH_CLIENT_ID', $tu_oauth_client->fields['identifier']);
 define('TU_OAUTH_CLIENT_SECRET', $tu_oauth_client->fields['secret']);
-
-// There is no need to pollute the output with error messages.
-ini_set('display_errors', 'Off');
-ErrorHandler::getInstance()->disableOutput();
-ErrorHandler::getInstance()->setForwardToInternalHandler(false);

phpunit/functional/Glpi/Controller/ErrorControllerTest.php

@@ -42,7 +42,6 @@
 use Glpi\Exception\Http\HttpException;
 use Glpi\Exception\Http\NotFoundHttpException;
 use PHPUnit\Framework\Attributes\DataProvider;
-use Psr\Log\LogLevel;
 use Session;
 use Symfony\Component\HttpFoundation\Request;
 
@@ -182,10 +181,6 @@ public function testErrorPageRendering(
         } else {
             $this->assertStringNotContainsString('<pre data-testid="stack-trace">', $content);
         }
-
-        if ($expected_code >= 500) {
-            $this->hasPhpLogRecordThatContains('*** Uncaught Exception', LogLevel::CRITICAL);
-        }
     }
 
     #[DataProvider('requestProvider')]
@@ -226,10 +221,6 @@ public function testErrorBlockRendering(
         } else {
             $this->assertStringNotContainsString('<pre data-testid="stack-trace">', $content);
         }
-
-        if ($expected_code >= 500) {
-            $this->hasPhpLogRecordThatContains('*** Uncaught Exception', LogLevel::CRITICAL);
-        }
     }
 
     #[DataProvider('requestProvider')]
@@ -278,9 +269,5 @@ public function testErrorJsonRendering(
 
         $this->assertArrayHasKey('trace', $decoded_content);
         $this->assertEquals($debug_mode, $decoded_content['trace'] !== null);
-
-        if ($expected_code >= 500) {
-            $this->hasPhpLogRecordThatContains('*** Uncaught Exception', LogLevel::CRITICAL);
-        }
     }
 }