From 6ee0e6235a0f1bbb094848d018f161c96113a589 Mon Sep 17 00:00:00 2001 From: Alexandros-Pallis Date: Mon, 14 Sep 2026 13:34:17 +0300 Subject: [PATCH] fix(Logger): keep running remaining handlers after one returns false Logger::log() stopped executing any subsequent handlers as soon as one handler's handle() returned false. In practice every shipped handler (FileHandler, ErrorlogHandler) returns false purely to signal a write failure, not a deliberate "stop the chain" request. This meant a single FileHandler failure (e.g. bad file permissions) silently swallowed the log message for every handler configured after it, such as an ErrorlogHandler fallback. None of the shipped handlers rely on the early-exit behavior, so drop it: every configured, level-matching handler now runs regardless of what earlier handlers returned. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GQ1FHzXX7To34G4mwQGN8U --- system/Log/Handlers/HandlerInterface.php | 7 ++--- system/Log/Logger.php | 5 +-- tests/_support/Log/Handlers/TestHandler.php | 3 -- tests/system/Log/LoggerTest.php | 34 +++++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 1 + 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/system/Log/Handlers/HandlerInterface.php b/system/Log/Handlers/HandlerInterface.php index 40a9958c714a..4f3bdd04bd69 100644 --- a/system/Log/Handlers/HandlerInterface.php +++ b/system/Log/Handlers/HandlerInterface.php @@ -19,10 +19,9 @@ interface HandlerInterface { /** - * Handles logging the message. - * If the handler returns false, then execution of handlers - * will stop. Any handlers that have not run, yet, will not - * be run. + * Handles logging the message. All configured handlers that + * can handle the given level are run, regardless of whether + * this (or any other) handler returns false. * * @param string $level * @param string $message diff --git a/system/Log/Logger.php b/system/Log/Logger.php index 6b61837fa22b..44b5778fbe7f 100644 --- a/system/Log/Logger.php +++ b/system/Log/Logger.php @@ -266,10 +266,7 @@ public function log($level, string|Stringable $message, array $context = []): vo continue; } - // If the handler returns false, then we don't execute any other handlers. - if (! $handler->setDateFormat($this->dateFormat)->handle($level, $message)) { - break; - } + $handler->setDateFormat($this->dateFormat)->handle($level, $message); } } diff --git a/tests/_support/Log/Handlers/TestHandler.php b/tests/_support/Log/Handlers/TestHandler.php index 025943ffe588..4087d16084a3 100644 --- a/tests/_support/Log/Handlers/TestHandler.php +++ b/tests/_support/Log/Handlers/TestHandler.php @@ -50,9 +50,6 @@ public function __construct(array $config) /** * Handles logging the message. - * If the handler returns false, then execution of handlers - * will stop. Any handlers that have not run, yet, will not - * be run. * * @param string $level * @param string $message diff --git a/tests/system/Log/LoggerTest.php b/tests/system/Log/LoggerTest.php index bbdf304acf8c..c99e0c19e089 100644 --- a/tests/system/Log/LoggerTest.php +++ b/tests/system/Log/LoggerTest.php @@ -18,6 +18,7 @@ use CodeIgniter\Exceptions\RuntimeException; use CodeIgniter\I18n\Time; use CodeIgniter\Log\Exceptions\LogException; +use CodeIgniter\Log\Handlers\BaseHandler; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockLogger as LoggerConfig; use PHPUnit\Framework\Attributes\Group; @@ -108,6 +109,39 @@ public function testLogDoesnotLogUnhandledLevels(): void $this->assertCount(0, $logs); } + public function testLogRunsRemainingHandlersWhenAnEarlierHandlerReturnsFalse(): void + { + $config = new LoggerConfig(); + + $failingHandler = new class ([]) extends BaseHandler { + public static int $timesCalled = 0; + + public function handle($level, $message): bool + { + self::$timesCalled++; + + return false; + } + }; + + $config->handlers = [ + $failingHandler::class => [ + 'handles' => ['debug'], + ], + TestHandler::class => [ + 'handles' => ['debug'], + 'path' => '', + ], + ]; + + $logger = new Logger($config); + + $logger->log('debug', 'Test message'); + + $this->assertSame(1, $failingHandler::$timesCalled); + $this->assertCount(1, TestHandler::getLogs()); + } + public function testLogInterpolatesMessage(): void { $config = new LoggerConfig(); diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 847a8d751a81..61793b320615 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -52,6 +52,7 @@ Bugs Fixed - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden). - **I18n:** Fixed a bug where ``Time::today()``, ``Time::yesterday()``, and ``Time::tomorrow()`` ignored the specified ``$timezone`` and ``setTestNow()`` when calculating the day. - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. +- **Logger:** Fixed a bug where ``Logger::log()`` stopped running any remaining configured handlers after one handler returned ``false`` (e.g., ``FileHandler`` failing to open its log file due to file permissions), silently dropping the log message for every handler after it. - **Cache:** Fixed ``MemcachedHandler::decrement()`` initializing a non-existent counter to the positive offset. Missing counters are now initialized to ``0``, reflecting Memcached's unsigned, saturating counter semantics. See the repo's