diff --git a/dependency_injection/services.php b/dependency_injection/services.php
index feb0727550f..7408857b162 100644
--- a/dependency_injection/services.php
+++ b/dependency_injection/services.php
@@ -36,7 +36,6 @@
use Glpi\DependencyInjection\PublicService;
use Glpi\Error\ErrorHandler;
-use Glpi\Log\LegacyGlobalLogger;
return static function (ContainerConfigurator $container): void {
$projectDir = dirname(__DIR__);
@@ -66,5 +65,5 @@
* Override Symfony's logger.
* @see \Symfony\Component\HttpKernel\DependencyInjection\LoggerPass
*/
- $services->set('logger', LegacyGlobalLogger::class);
+ $services->set('logger')->synthetic();
};
diff --git a/phpunit/functional/Glpi/Exception/LogLineFormatterTest.php b/phpunit/functional/Glpi/Exception/LogLineFormatterTest.php
index 9e2dea6b9f5..a3afb538ed1 100644
--- a/phpunit/functional/Glpi/Exception/LogLineFormatterTest.php
+++ b/phpunit/functional/Glpi/Exception/LogLineFormatterTest.php
@@ -34,7 +34,7 @@
namespace tests\units\Glpi\Exception;
-use Glpi\Error\LogLineFormatter;
+use Glpi\Log\LogLineFormatter;
use PHPUnit\Framework\TestCase;
class LogLineFormatterTest extends TestCase
diff --git a/src/CommonGLPI.php b/src/CommonGLPI.php
index 32d860ba8d9..b810450be56 100644
--- a/src/CommonGLPI.php
+++ b/src/CommonGLPI.php
@@ -1197,7 +1197,6 @@ public function getHeaderName(): string
*/
public function display($options = [])
{
- trigger_error('Error triggerred', E_USER_WARNING);
// Item might already be loaded, skip load and rights checks
$item_loaded = $options['loaded'] ?? false;
unset($options['loaded']);
diff --git a/src/GLPI.php b/src/GLPI.php
index 56ab391064b..68f48ad171d 100644
--- a/src/GLPI.php
+++ b/src/GLPI.php
@@ -33,7 +33,7 @@
* ---------------------------------------------------------------------
*/
-use Glpi\Error\LogLineFormatter;
+use Glpi\Log\LogLineFormatter;
use Monolog\Handler\StreamHandler;
use Monolog\Logger;
use Psr\Log\LogLevel;
@@ -66,50 +66,4 @@ class GLPI
*/
public const ENV_DEVELOPMENT = 'development';
- /**
- * Init logger
- *
- * @return void
- */
- public function initLogger()
- {
- $errorHandler = new \Glpi\Error\ErrorHandler();
- $errorHandler::register($errorHandler);
-
- /**
- * @var \Psr\Log\LoggerInterface $PHPLOGGER
- */
- global $PHPLOGGER;
-
- if (defined('GLPI_LOG_LVL')) {
- $log_level = GLPI_LOG_LVL;
- } else {
- switch (GLPI_ENVIRONMENT_TYPE) {
- case self::ENV_DEVELOPMENT:
- // All error/messages are logs, including deprecations.
- $log_level = LogLevel::DEBUG;
- break;
- case self::ENV_TESTING:
- // Silent deprecation and info, as they should have no functional impact.
- // Keep notices as they have may indicate that code is not correctly handling a specific case.
- $log_level = LogLevel::NOTICE;
- break;
- case self::ENV_STAGING:
- case self::ENV_PRODUCTION:
- default:
- // Keep only warning/error messages.
- $log_level = LogLevel::WARNING;
- break;
- }
- }
-
- $PHPLOGGER = new Logger('glpiphplog');
- $handler = new StreamHandler(
- GLPI_LOG_DIR . "/php-errors.log",
- $log_level
- );
- $handler->setFormatter(new LogLineFormatter());
-
- $PHPLOGGER->pushHandler($handler);
- }
}
diff --git a/src/Glpi/Application/SystemConfigurator.php b/src/Glpi/Application/SystemConfigurator.php
index d3c2641e1ba..cb78665dd77 100644
--- a/src/Glpi/Application/SystemConfigurator.php
+++ b/src/Glpi/Application/SystemConfigurator.php
@@ -35,16 +35,25 @@
namespace Glpi\Application;
+use Glpi\Error\ErrorHandler;
+use Glpi\Log\GlpiLogHandler;
+use Monolog\Logger;
+use Psr\Log\LoggerInterface;
+
final class SystemConfigurator
{
+ private LoggerInterface $logger;
+
public function __construct(private string $root_dir, private ?string $env)
{
+ $this->computeConstants();
+ $this->setSessionConfiguration();
+ $this->initLogger();
}
- public function __invoke(): void
+ public function getLogger(): LoggerInterface
{
- $this->computeConstants();
- $this->setSessionConfiguration();
+ return $this->logger;
}
private function computeConstants(): void
@@ -245,4 +254,21 @@ private function setSessionConfiguration(): void
// for every GLPI instance, enven if they are served by the same server (mostly for dev envs).
session_name('glpi_' . \hash('sha512', $this->root_dir . ($_SERVER['HTTP_HOST'] ?? '') . ($_SERVER['SERVER_PORT'] ?? '')));
}
+
+ private function initLogger(): void
+ {
+ /**
+ * @var LoggerInterface $PHPLOGGER
+ */
+ global $PHPLOGGER;
+
+ $PHPLOGGER = new Logger('glpi');
+ $PHPLOGGER->pushHandler(new GlpiLogHandler());
+
+ $this->logger = $PHPLOGGER;
+
+ $errorHandler = new ErrorHandler($PHPLOGGER);
+ $errorHandler::register($errorHandler);
+
+ }
}
diff --git a/src/Glpi/Controller/CentralController.php b/src/Glpi/Controller/CentralController.php
index c64149db06b..647bffbf9c4 100644
--- a/src/Glpi/Controller/CentralController.php
+++ b/src/Glpi/Controller/CentralController.php
@@ -69,6 +69,8 @@ public function __invoke(Request $request): Response
return new RedirectResponse($url);
}
+ throw new \RuntimeException('Noop');
+
return $this->render('pages/central/index.html.twig', [
'central' => new Central(),
]);
diff --git a/src/Glpi/Controller/ErrorController.php b/src/Glpi/Controller/ErrorController.php
index 3aae8103f44..6d68fa4c736 100644
--- a/src/Glpi/Controller/ErrorController.php
+++ b/src/Glpi/Controller/ErrorController.php
@@ -36,7 +36,6 @@
use Config;
use DBConnection;
-use Glpi\Application\ErrorUtils;
use Html;
use Session;
use Symfony\Component\ErrorHandler\Error\OutOfMemoryError;
@@ -57,12 +56,17 @@ public function __invoke(Request $request, ?\Throwable $exception = null): Respo
return new Response('', 500);
}
- $this->logException($exception, $request);
+ $this->logHttpException($exception, $request);
return $this->getErrorResponse($exception, $request);
}
- private function logException(\Throwable $exception, Request $request): void
+ /**
+ * @TODO: create a specific handler in our logger so that HTTP errors are logged differently according to this method ⬇
+ *
+ * @see \Glpi\Log\GlpiLogHandler::canHandle
+ */
+ private function logHttpException(\Throwable $exception, Request $request): void
{
if (
$exception instanceof HttpExceptionInterface
@@ -78,29 +82,23 @@ private function logException(\Throwable $exception, Request $request): void
$user_id = Session::getLoginUserID() ?: 'Anonymous';
- switch ($exception::class) {
- case AccessDeniedHttpException::class:
- $message = sprintf(
- 'User ID: `%s` tried to access or perform an action on `%s` with insufficient rights.',
- $user_id,
- $requested_uri
- );
- break;
- case NotFoundHttpException::class:
- $message = sprintf(
- 'User ID: `%s` tried to access a non-existent item on `%s`.',
- $user_id,
- $requested_uri
- );
- break;
- default:
- $message = sprintf(
- 'User ID: `%s` tried to execute an invalid request on `%s`.',
- $user_id,
- $requested_uri
- );
- break;
- }
+ $message = match ($exception::class) {
+ AccessDeniedHttpException::class => sprintf(
+ 'User ID: `%s` tried to access or perform an action on `%s` with insufficient rights.',
+ $user_id,
+ $requested_uri
+ ),
+ NotFoundHttpException::class => sprintf(
+ 'User ID: `%s` tried to access a non-existent item on `%s`.',
+ $user_id,
+ $requested_uri
+ ),
+ default => sprintf(
+ 'User ID: `%s` tried to execute an invalid request on `%s`.',
+ $user_id,
+ $requested_uri
+ ),
+ };
if (($exception_message = $exception->getMessage()) !== '') {
$message .= sprintf('Additional information: %s', $exception_message);
@@ -119,10 +117,6 @@ private function logException(\Throwable $exception, Request $request): void
}
Toolbox::logInFile('access-errors', $message);
- } else {
- // Other errors are logged in the `php-errors` log
- ErrorUtils::logException($exception);
- ErrorUtils::outputExceptionMessage($exception);
}
}
diff --git a/src/Glpi/Error/ErrorDisplayHandler/HtmlErrorDisplayHandler.php b/src/Glpi/Error/ErrorDisplayHandler/HtmlErrorDisplayHandler.php
index 0a9412f542f..08c9983e8ed 100644
--- a/src/Glpi/Error/ErrorDisplayHandler/HtmlErrorDisplayHandler.php
+++ b/src/Glpi/Error/ErrorDisplayHandler/HtmlErrorDisplayHandler.php
@@ -47,20 +47,30 @@ public static function setCurrentRequest(Request $request): void
public function canOutput(string $log_level, string $env): bool
{
- return self::$currentRequest !== null;
+ if (!self::$currentRequest) {
+ return false;
+ }
+ $is_dev_env = $env === \GLPI::ENV_DEVELOPMENT;
+ $is_debug_mode = isset($_SESSION['glpi_use_mode']) && $_SESSION['glpi_use_mode'] == \Session::DEBUG_MODE;
+
+ if (
+ !$is_dev_env // error messages are always displayed in development environment
+ && !$is_debug_mode // error messages are always displayed in debug mode
+ ) {
+ return false;
+ }
+
+ /**
+ * @see Request::initializeFormats
+ */
+ $type = self::$currentRequest->getPreferredFormat();
+
+ return $type === 'html';
}
public function displayErrorMessage(string $error_type, string $message, string $log_level, mixed $env): void
{
- $req = self::$currentRequest;
-
- $types = $req->getAcceptableContentTypes();
-
- if (in_array('text/html', $types)) {
- echo ''
- . '' . \htmlescape($error_type) . ': ' . \htmlescape($message) . '
';
- } else {
- \trigger_error('Cannot display error in HTTP context for requests that do not accept HTML as a response.', \E_USER_WARNING);
- }
+ echo ''
+ . '' . \htmlescape($error_type) . ': ' . \htmlescape($message) . '
';
}
}
diff --git a/src/Glpi/Error/ErrorDisplayHandler/EchoDisplayHandler.php b/src/Glpi/Error/ErrorDisplayHandler/LegacyCliDisplayHandler.php
similarity index 93%
rename from src/Glpi/Error/ErrorDisplayHandler/EchoDisplayHandler.php
rename to src/Glpi/Error/ErrorDisplayHandler/LegacyCliDisplayHandler.php
index 1819dc1de35..c2644036c4a 100644
--- a/src/Glpi/Error/ErrorDisplayHandler/EchoDisplayHandler.php
+++ b/src/Glpi/Error/ErrorDisplayHandler/LegacyCliDisplayHandler.php
@@ -34,11 +34,11 @@
namespace Glpi\Error\ErrorDisplayHandler;
-final class EchoDisplayHandler implements ErrorDisplayHandler
+final class LegacyCliDisplayHandler implements ErrorDisplayHandler
{
public function canOutput(string $log_level, string $env): bool
{
- return true;
+ return \isCommandLine();
}
public function displayErrorMessage(string $error_type, string $message, string $log_level, mixed $env): void
diff --git a/src/Glpi/Error/ErrorHandler.php b/src/Glpi/Error/ErrorHandler.php
index b36ad4932f3..ed0f415dc28 100644
--- a/src/Glpi/Error/ErrorHandler.php
+++ b/src/Glpi/Error/ErrorHandler.php
@@ -36,13 +36,12 @@
use GLPI;
use Glpi\Error\ErrorDisplayHandler\ConsoleErrorDisplayHandler;
-use Glpi\Error\ErrorDisplayHandler\EchoDisplayHandler;
+use Glpi\Error\ErrorDisplayHandler\LegacyCliDisplayHandler;
use Glpi\Error\ErrorDisplayHandler\HtmlErrorDisplayHandler;
use Glpi\Error\ErrorDisplayHandler\ErrorDisplayHandler;
use Monolog\Logger;
+use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
-use Symfony\Component\Console\Output\ConsoleOutput;
-use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\ErrorHandler\BufferingLogger;
use Symfony\Component\ErrorHandler\ErrorHandler as BaseErrorHandler;
@@ -91,21 +90,21 @@ final class ErrorHandler extends BaseErrorHandler
*/
private static bool $enable_output = true;
- private static Logger $currentLogger;
+ private static LoggerInterface $currentLogger;
private string $env;
- public function __construct(?BufferingLogger $bootstrappingLogger = null, bool $debug = false, string $env = \GLPI_ENVIRONMENT_TYPE)
+ public function __construct(LoggerInterface $logger)
{
- parent::__construct($bootstrappingLogger, $debug);
+ parent::__construct();
- $this->env = $env;
+ $this->env = \GLPI_ENVIRONMENT_TYPE;
$this->scopeAt(self::FATAL_ERRORS, true);
$this->screamAt(self::FATAL_ERRORS, true);
$this->traceAt(self::FATAL_ERRORS, true);
$this->throwAt(self::FATAL_ERRORS, true);
- $this->configureLogger();
+ self::$currentLogger = $logger;
$this->configureErrorDisplay();
}
@@ -119,12 +118,8 @@ public static function disableOutput(): void
self::$enable_output = false;
}
- public static function getCurrentLogger(): Logger
+ public static function getCurrentLogger(): LoggerInterface
{
- if (!self::$currentLogger) {
- throw new \RuntimeException('Logger was fetched from an unbuilt ErrorHandler, probably because the call was made before Kernel was created. Please check your GLPI codebase is up-to-date and that all your PHP files are executed within a valid Kernel.');
- }
-
return self::$currentLogger;
}
@@ -143,7 +138,7 @@ private static function getOutputHandlers(): array
return $handlers = [
new ConsoleErrorDisplayHandler(),
new HtmlErrorDisplayHandler(),
- new EchoDisplayHandler(),
+ new LegacyCliDisplayHandler(),
];
}
@@ -153,31 +148,12 @@ public static function displayErrorMessage(string $error_type, string $message,
return;
}
- $is_dev_env = $env === \GLPI::ENV_DEVELOPMENT;
- $is_debug_mode = isset($_SESSION['glpi_use_mode']) && $_SESSION['glpi_use_mode'] == \Session::DEBUG_MODE;
- $is_console_context = \isCommandLine();
-
- if (
- !$is_dev_env // error messages are always displayed in development environment
- && !$is_debug_mode // error messages are always displayed in debug mode
- && !$is_console_context // error messages are always forwarded to the console output handler, that handles itself the verbosity level
- ) {
- return;
- }
-
- $output_handled = false;
-
foreach (self::getOutputHandlers() as $handler) {
if ($handler->canOutput($log_level, $env)) {
- $output_handled = true;
$handler->displayErrorMessage($error_type, $message, $log_level, $env);
break; // Only one display per handler
}
}
-
- if (!$output_handled) {
- throw new \RuntimeException('No error display handler was able to output the error message.');
- }
}
public function handleError(int $type, string $message, string $file, int $line): bool
@@ -196,31 +172,6 @@ public function handleError(int $type, string $message, string $file, int $line)
return $handled;
}
- private function configureLogger(): void
- {
- if (\defined('GLPI_LOG_LVL')) {
- $minimum_log_level = GLPI_LOG_LVL;
- } else {
- $minimum_log_level = match (\GLPI_ENVIRONMENT_TYPE) {
- \GLPI::ENV_DEVELOPMENT => LogLevel::DEBUG,
- \GLPI::ENV_TESTING => LogLevel::NOTICE,
- default => LogLevel::WARNING,
- };
- }
-
- $logger = new Logger('glpierrorlog');
- $handler = new \Monolog\Handler\StreamHandler(
- GLPI_LOG_DIR . "/glpi-errors.log",
- $minimum_log_level
- );
- $handler->setFormatter(new LogLineFormatter());
-
- $logger->pushHandler($handler);
- $this->setDefaultLogger($logger);
-
- self::$currentLogger = $logger;
- }
-
private function configureErrorDisplay(): void
{
// Adjust reporting level to the environment, to ensure that all the errors supposed to be logged are
diff --git a/src/Glpi/Kernel/Kernel.php b/src/Glpi/Kernel/Kernel.php
index aae98d67cd5..2b659bc1a51 100644
--- a/src/Glpi/Kernel/Kernel.php
+++ b/src/Glpi/Kernel/Kernel.php
@@ -37,6 +37,7 @@
use GLPI;
use Glpi\Application\SystemConfigurator;
use Glpi\Http\Listener\PluginsRouterListener;
+use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Bundle\TwigBundle\TwigBundle;
@@ -53,16 +54,15 @@ final class Kernel extends BaseKernel
{
use MicroKernelTrait;
+ private LoggerInterface $logger;
+
public function __construct(?string $env = null)
{
// Initialize system configuration.
// It must be done after the autoload inclusion that requires some constants to be defined (e.g. GLPI_VERSION).
// It must be done before the Kernel boot as some of the define constants must be defined during the boot sequence.
- (new SystemConfigurator($this->getProjectDir(), $env))();
-
- // TODO: refactor the GLPI class.
- $glpi = (new GLPI());
- $glpi->initLogger();
+ $configurator = new SystemConfigurator($this->getProjectDir(), $env);
+ $this->logger = $configurator->getLogger();
$env = GLPI_ENVIRONMENT_TYPE;
parent::__construct(
@@ -113,6 +113,9 @@ public function boot(): void
parent::boot();
+ // Override Symfony's logger
+ $this->container->set('logger', $this->logger);
+
if ($dispatch_postboot) {
$this->container->get('event_dispatcher')->dispatch(new PostBootEvent());
}
diff --git a/src/Glpi/Log/GlpiLogHandler.php b/src/Glpi/Log/GlpiLogHandler.php
new file mode 100644
index 00000000000..0318776e41a
--- /dev/null
+++ b/src/Glpi/Log/GlpiLogHandler.php
@@ -0,0 +1,107 @@
+.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace Glpi\Log;
+
+use Monolog\Handler\StreamHandler;
+use Monolog\Level;
+use Monolog\LogRecord;
+use Psr\Log\LogLevel;
+use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
+
+class GlpiLogHandler extends StreamHandler
+{
+ public function __construct()
+ {
+ if (\defined('GLPI_LOG_LVL')) {
+ $log_level = GLPI_LOG_LVL;
+ } else {
+ $log_level = match (GLPI_ENVIRONMENT_TYPE) {
+ \GLPI::ENV_DEVELOPMENT => LogLevel::DEBUG,
+ \GLPI::ENV_TESTING => LogLevel::NOTICE,
+ default => LogLevel::WARNING,
+ };
+ }
+
+ parent::__construct(GLPI_LOG_DIR . '/php-errors.log', $log_level);
+
+ $this->setFormatter(new LogLineFormatter());
+ }
+
+ public function isHandling(LogRecord $record): bool
+ {
+ if (!$this->canHandle($record)) {
+ return false;
+ }
+
+ return parent::isHandling($record);
+ }
+
+ public function handle(LogRecord $record): bool
+ {
+ if (!$this->canHandle($record)) {
+ return false;
+ }
+
+ return parent::handle($record);
+ }
+
+ /**
+ * The goal here is mostly to disable logging Symfony's Kernel events.
+ * It avoid flooding the debug logs with "Notified event {...}" messages.
+ */
+ public function canHandle(LogRecord $record): bool
+ {
+ if (isset($record->context['event']) && $record->level === Level::Debug) {
+ return false;
+ }
+
+ /**
+ * @see \Glpi\Controller\ErrorController::logHttpException
+ */
+ if (isset($record->context['exception'])) {
+ /** @var \Throwable $exception */
+ $exception = $record->context['exception'];
+ if (
+ $exception instanceof HttpExceptionInterface
+ && $exception->getStatusCode() >= 400
+ && $exception->getStatusCode() < 500
+ ) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+}
diff --git a/src/Glpi/Log/LegacyGlobalLogger.php b/src/Glpi/Log/LegacyGlobalLogger.php
deleted file mode 100644
index 903e56ac020..00000000000
--- a/src/Glpi/Log/LegacyGlobalLogger.php
+++ /dev/null
@@ -1,106 +0,0 @@
-.
- *
- * ---------------------------------------------------------------------
- */
-
-namespace Glpi\Log;
-
-use Glpi\Exception\Http\HttpExceptionInterface;
-use Psr\Log\LoggerInterface;
-use Psr\Log\NullLogger;
-
-class LegacyGlobalLogger implements LoggerInterface
-{
- private function getLogger(): LoggerInterface
- {
- /**
- * @var null|\Psr\Log\LoggerInterface $PHPLOGGER
- */
- global $PHPLOGGER;
-
- return $PHPLOGGER ?? new NullLogger();
- }
-
- public function emergency(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->emergency($message, $context);
- }
-
- public function alert(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->alert($message, $context);
- }
-
- public function critical(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->critical($message, $context);
- }
-
- public function error(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->error($message, $context);
- }
-
- public function warning(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->warning($message, $context);
- }
-
- public function notice(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->notice($message, $context);
- }
-
- public function info(\Stringable|string $message, array $context = []): void
- {
- $this->getLogger()->info($message, $context);
- }
-
- public function debug(\Stringable|string $message, array $context = []): void
- {
- // Debug is disabled for now because Symfony generates a lot of debug logs that GLPI doesn't need yet.
- }
-
- public function log($level, \Stringable|string $message, array $context = []): void
- {
- if (isset($context['exception']) && $context['exception'] instanceof HttpExceptionInterface) {
- // Bypass \Symfony\Component\HttpKernel\EventListener\ErrorListener's logging system.
- // Symfony is force-logging all exceptions, even the deliberately thrown HTTP exceptions.
- // In GLPI, 4xx exception are not usually an "issue", and are logged in the "access" channel.
- // However, Symfony logs them in the "error" channel.
- // To keep most of Symfony's behavior in non-HTTP exceptions, we just remove the HttpExceptionInterface from it.
- return;
- }
-
- $this->getLogger()->log($level, $message, $context);
- }
-}
diff --git a/src/Glpi/Error/LogLineFormatter.php b/src/Glpi/Log/LogLineFormatter.php
similarity index 97%
rename from src/Glpi/Error/LogLineFormatter.php
rename to src/Glpi/Log/LogLineFormatter.php
index 5f694785202..f0976c18151 100644
--- a/src/Glpi/Error/LogLineFormatter.php
+++ b/src/Glpi/Log/LogLineFormatter.php
@@ -32,8 +32,9 @@
* ---------------------------------------------------------------------
*/
-namespace Glpi\Error;
+namespace Glpi\Log;
+use Glpi\Error\StackTraceFormatter;
use Monolog\Formatter\LineFormatter;
final class LogLineFormatter extends LineFormatter