fix(Logger): keep running remaining handlers after one returns false

[Alexandros-Pallis]

As a user of the framework this seems like a bug to me, i had 2 handlers defined in my $handlers config ( FileHandler and ErrorLogHandler ). The file handler failed due to file permissions on my system and the ErrorLogHandler was not working at all which confused me.

I've read that you prefer a PR rather than an Issue on the contributing docs and decided to open a PR instead.

I used AI for the fix but i have manually checked the generated code and run the tests.

Let me know if you don't consider this a bug or if i should change anything in order to be acceptable.

Description

Logger::log() stops iterating handlers as soon as one handler's handle() call returns false:

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

In practice, every shipped handler (FileHandler, ErrorlogHandler) returns false purely as a write-failure signal, not as a deliberate "stop the chain" request. This means a single FileHandler failure (e.g. bad file permissions on the log directory) silently swallows the log message for every handler configured after it — for example an ErrorlogHandler fallback never runs, and the log entry is lost with no indication why.

I ran into this directly: FileHandler + ErrorlogHandler configured in that order, FileHandler failed due to permissions, and ErrorlogHandler never got a chance to log anything.

Fix

Drop the early-exit: every configured handler that can handle the given level now runs, regardless of what earlier handlers returned. No shipped handler relies on the early-exit behavior (confirmed by checking FileHandler, ErrorlogHandler, and ChromeLoggerHandler — none use their return value to intentionally halt the chain), so this only changes behavior for the failure case, not any documented feature.

HandlerInterface::handle()'s docblock is updated to reflect the new contract.

Testing

• Added LoggerTest::testLogRunsRemainingHandlersWhenAnEarlierHandlerReturnsFalse(), using an inline anonymous handler that always returns false, configured before TestHandler, to assert the second handler still receives the log message.
• Full tests/system/Log/ suite passes (44 tests, 78 assertions).
• phpstan analyse and php-cs-fixer --dry-run clean on all touched files.

Checklist

• I have read the CONTRIBUTING guide
• My code follows the code style of this project
• My change requires a change to the documentation
• I have updated the documentation accordingly
• This PR is on a custom branch and not develop

🤖 Generated with Claude Code

[mergeable]

Hi there, Alexandros-Pallis! 👋

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

• PRs must be sent to the appropriate branch
• All git commits must be GPG-signed
• Must follow our style guide
• Be commented in the PHP source file
• Be documented in the user guide
• Be unit tested
• Pass all checks in GitHub Actions

Important

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work
on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Sincerely, the mergeable bot 🤖

[michalsn]

I understand why this behavior was surprising. However, HandlerInterface::handle() explicitly documents that returning false stops subsequent handlers, so this changes the existing API contract rather than fixes a bug. We can consider changing that behavior, but it should be proposed for the 4.8 branch, with backward compatibility and migration guidance addressed.

[Alexandros-Pallis]

I understand why this behavior was surprising. However, HandlerInterface::handle() explicitly documents that returning false stops subsequent handlers, so this changes the existing API contract rather than fixes a bug. We can consider changing that behavior, but it should be proposed for the 4.8 branch, with backward compatibility and migration guidance addressed.

I saw that this behavior is documented, i am not fully grasping the reason of this behavior. I can bypass my issue by rearranging the $handlers inside the config and putting FileHandler last.

If you decide that this behavior should change, let me know if i can do anything additional to help towards this direction.

Thanks for the feedback!

[michalsn]

Let's see what others think.

[paulbalandan]

If we'll target 4.8 for a breaking change, what if we change the boolean return to an int return? Then, any nonzero exit can be decided whether the next handlers in queue to continue logging or abort completely. Like 1 is continue and 2 is terminate.

[michalsn]

If we'll target 4.8 for a breaking change, what if we change the boolean return to an int return? Then, any nonzero exit can be decided whether the next handlers in queue to continue logging or abort completely. Like 1 is continue and 2 is terminate.

That would be a big change, but probably acceptable, as not many people use their own handlers.

[michalsn]

@Alexandros-Pallis, if you're interested in implementing this the way Paul described, you're welcome to send a PR targeting the 4.8 branch.