Skip to content

Commit 5d2635e

Browse files
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQ1FHzXX7To34G4mwQGN8U
1 parent 01fd6ee commit 5d2635e

6 files changed

Lines changed: 75 additions & 11 deletions

File tree

system/Log/Handlers/HandlerInterface.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@
1919
interface HandlerInterface
2020
{
2121
/**
22-
* Handles logging the message.
23-
* If the handler returns false, then execution of handlers
24-
* will stop. Any handlers that have not run, yet, will not
25-
* be run.
22+
* Handles logging the message. All configured handlers that
23+
* can handle the given level are run, regardless of whether
24+
* this (or any other) handler returns false.
2625
*
2726
* @param string $level
2827
* @param string $message

system/Log/Logger.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,7 @@ public function log($level, string|Stringable $message, array $context = []): vo
266266
continue;
267267
}
268268

269-
// If the handler returns false, then we don't execute any other handlers.
270-
if (! $handler->setDateFormat($this->dateFormat)->handle($level, $message)) {
271-
break;
272-
}
269+
$handler->setDateFormat($this->dateFormat)->handle($level, $message);
273270
}
274271
}
275272

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace Tests\Support\Log\Handlers;
15+
16+
use CodeIgniter\Log\Handlers\BaseHandler;
17+
18+
/**
19+
* A LogHandler that always fails to handle the log message.
20+
* Only used for testing purposes.
21+
*/
22+
class FailingTestHandler extends BaseHandler
23+
{
24+
/**
25+
* Number of times handle() has been called.
26+
*/
27+
protected static int $timesCalled = 0;
28+
29+
public function __construct(array $config)
30+
{
31+
parent::__construct($config);
32+
33+
self::$timesCalled = 0;
34+
}
35+
36+
public function handle($level, $message): bool
37+
{
38+
self::$timesCalled++;
39+
40+
return false;
41+
}
42+
43+
public static function getTimesCalled(): int
44+
{
45+
return self::$timesCalled;
46+
}
47+
}

tests/_support/Log/Handlers/TestHandler.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ public function __construct(array $config)
5050

5151
/**
5252
* Handles logging the message.
53-
* If the handler returns false, then execution of handlers
54-
* will stop. Any handlers that have not run, yet, will not
55-
* be run.
5653
*
5754
* @param string $level
5855
* @param string $message

tests/system/Log/LoggerTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use ReflectionNamedType;
2626
use stdClass;
2727
use Stringable;
28+
use Tests\Support\Log\Handlers\FailingTestHandler;
2829
use Tests\Support\Log\Handlers\TestHandler;
2930

3031
/**
@@ -108,6 +109,28 @@ public function testLogDoesnotLogUnhandledLevels(): void
108109
$this->assertCount(0, $logs);
109110
}
110111

112+
public function testLogRunsRemainingHandlersWhenAnEarlierHandlerReturnsFalse(): void
113+
{
114+
$config = new LoggerConfig();
115+
116+
$config->handlers = [
117+
FailingTestHandler::class => [
118+
'handles' => ['debug'],
119+
],
120+
TestHandler::class => [
121+
'handles' => ['debug'],
122+
'path' => '',
123+
],
124+
];
125+
126+
$logger = new Logger($config);
127+
128+
$logger->log('debug', 'Test message');
129+
130+
$this->assertSame(1, FailingTestHandler::getTimesCalled());
131+
$this->assertCount(1, TestHandler::getLogs());
132+
}
133+
111134
public function testLogInterpolatesMessage(): void
112135
{
113136
$config = new LoggerConfig();

user_guide_src/source/changelogs/v4.7.5.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ Bugs Fixed
5252
- **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden).
5353
- **I18n:** Fixed a bug where ``Time::today()``, ``Time::yesterday()``, and ``Time::tomorrow()`` ignored the specified ``$timezone`` and ``setTestNow()`` when calculating the day.
5454
- **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors.
55+
- **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.
5556
- **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.
5657

5758
See the repo's

0 commit comments

Comments
 (0)