diff --git a/app/Config/Logger.php b/app/Config/Logger.php index b273cb093004..954930f60d03 100644 --- a/app/Config/Logger.php +++ b/app/Config/Logger.php @@ -125,7 +125,7 @@ class Logger extends BaseConfig * Handlers are executed in the order defined in this array, starting with * the handler on top and continuing down. * - * @var array, array|string>> + * @var array, array|string>> */ public array $handlers = [ /* @@ -170,6 +170,14 @@ class Logger extends BaseConfig * Specify a different destination here, if desired. */ 'path' => '', + + /* + * Whether a failed write stops the handlers defined after this one. + * + * Set this to false to let the remaining handlers log the message + * even when this handler could not write it. + */ + 'stopChainOnFailure' => true, ], /* @@ -197,6 +205,11 @@ class Logger extends BaseConfig // * class constants: `ErrorlogHandler::TYPE_OS` (0) or `ErrorlogHandler::TYPE_SAPI` (4) // */ // 'messageType' => 0, + // + // /* + // * Whether a failed write stops the handlers defined after this one. + // */ + // 'stopChainOnFailure' => true, // ], ]; } diff --git a/system/Log/Handlers/BaseHandler.php b/system/Log/Handlers/BaseHandler.php index 20dceb8144c7..3abc951ac218 100644 --- a/system/Log/Handlers/BaseHandler.php +++ b/system/Log/Handlers/BaseHandler.php @@ -35,11 +35,17 @@ abstract class BaseHandler implements HandlerInterface protected $dateFormat = 'Y-m-d H:i:s'; /** - * @param array{handles?: list} $config + * Whether a failed write stops the execution of the remaining handlers. + */ + protected bool $stopChainOnFailure = true; + + /** + * @param array{handles?: list, stopChainOnFailure?: bool} $config */ public function __construct(array $config) { - $this->handles = $config['handles'] ?? []; + $this->handles = $config['handles'] ?? []; + $this->stopChainOnFailure = $config['stopChainOnFailure'] ?? true; } /** diff --git a/system/Log/Handlers/ErrorlogHandler.php b/system/Log/Handlers/ErrorlogHandler.php index 52f9add8cb5d..f12cf43942cc 100644 --- a/system/Log/Handlers/ErrorlogHandler.php +++ b/system/Log/Handlers/ErrorlogHandler.php @@ -45,7 +45,7 @@ class ErrorlogHandler extends BaseHandler /** * Constructor. * - * @param array{handles?: list, messageType?: int} $config + * @param array{handles?: list, stopChainOnFailure?: bool, messageType?: int} $config */ public function __construct(array $config = []) { @@ -66,6 +66,9 @@ public function __construct(array $config = []) * will stop. Any handlers that have not run, yet, will not * be run. * + * A failed write returns false only when `stopChainOnFailure` + * is enabled, which is the default. + * * @param string $level * @param string $message * @param array $context @@ -78,7 +81,7 @@ public function handle($level, $message, array $context = []): bool $message = strtoupper($level) . ' --> ' . $message . "\n"; - return $this->errorLog($message, $this->messageType); + return $this->errorLog($message, $this->messageType) || ! $this->stopChainOnFailure; } /** diff --git a/system/Log/Handlers/FileHandler.php b/system/Log/Handlers/FileHandler.php index 99bdc2b20113..d88b85b3a455 100644 --- a/system/Log/Handlers/FileHandler.php +++ b/system/Log/Handlers/FileHandler.php @@ -45,7 +45,7 @@ class FileHandler extends BaseHandler protected $filePermissions; /** - * @param array{handles?: list, path?: string, fileExtension?: string, filePermissions?: int} $config + * @param array{handles?: list, stopChainOnFailure?: bool, path?: string, fileExtension?: string, filePermissions?: int} $config */ public function __construct(array $config = []) { @@ -69,6 +69,9 @@ public function __construct(array $config = []) * will stop. Any handlers that have not run, yet, will not * be run. * + * A failed write returns false only when `stopChainOnFailure` + * is enabled, which is the default. + * * @param string $level * @param string $message * @param array $context @@ -92,7 +95,7 @@ public function handle($level, $message, array $context = []): bool } if (! $fp = @fopen($filepath, 'ab')) { - return false; + return ! $this->stopChainOnFailure; } // Instantiating DateTime with microseconds appended to initial date is needed for proper support of this format @@ -130,6 +133,6 @@ public function handle($level, $message, array $context = []): bool @chmod($filepath, $this->filePermissions); } - return is_int($result); + return is_int($result) || ! $this->stopChainOnFailure; } } diff --git a/system/Test/Mock/MockFileLogger.php b/system/Test/Mock/MockFileLogger.php index 5a815b223196..e85dfdf6e26a 100644 --- a/system/Test/Mock/MockFileLogger.php +++ b/system/Test/Mock/MockFileLogger.php @@ -28,7 +28,7 @@ class MockFileLogger extends FileHandler public $destination; /** - * @param array{handles?: list, path?: string, fileExtension?: string, filePermissions?: int} $config + * @param array{handles?: list, stopChainOnFailure?: bool, path?: string, fileExtension?: string, filePermissions?: int} $config */ public function __construct(array $config) { diff --git a/system/Test/Mock/MockLogger.php b/system/Test/Mock/MockLogger.php index 411c1a66a6be..a749e506833f 100644 --- a/system/Test/Mock/MockLogger.php +++ b/system/Test/Mock/MockLogger.php @@ -83,7 +83,7 @@ class MockLogger extends Logger * Handlers are executed in the order defined in this array, starting with * the handler on top and continuing down. * - * @var array, array|string>> + * @var array, array|string>> */ public array $handlers = [ // File Handler diff --git a/tests/system/Log/Handlers/ErrorlogHandlerTest.php b/tests/system/Log/Handlers/ErrorlogHandlerTest.php index 7aa89ab2a933..44275ba801c3 100644 --- a/tests/system/Log/Handlers/ErrorlogHandlerTest.php +++ b/tests/system/Log/Handlers/ErrorlogHandlerTest.php @@ -47,8 +47,32 @@ public function testErrorLoggingAppendsContextAsJson(): void $this->assertTrue($logger->handle('error', 'Test message.', [HandlerInterface::GLOBAL_CONTEXT_KEY => ['foo' => 'bar']])); } + public function testHandleReturnsFalseOnFailedWriteByDefault(): void + { + $logger = $this->getMockedHandler(['handles' => ['error']]); + $logger->expects($this->once())->method('errorLog')->willReturn(false); + + $this->assertFalse($logger->handle('error', 'Test message.')); + } + + public function testHandleReturnsFalseOnFailedWriteWhenStopChainOnFailureIsEnabled(): void + { + $logger = $this->getMockedHandler(['handles' => ['error'], 'stopChainOnFailure' => true]); + $logger->expects($this->once())->method('errorLog')->willReturn(false); + + $this->assertFalse($logger->handle('error', 'Test message.')); + } + + public function testHandleReturnsTrueOnFailedWriteWhenStopChainOnFailureIsDisabled(): void + { + $logger = $this->getMockedHandler(['handles' => ['error'], 'stopChainOnFailure' => false]); + $logger->expects($this->once())->method('errorLog')->willReturn(false); + + $this->assertTrue($logger->handle('error', 'Test message.')); + } + /** - * @param array{handles?: list, messageType?: int} $config + * @param array{handles?: list, stopChainOnFailure?: bool, messageType?: int} $config * * @return ErrorlogHandler&MockObject */ diff --git a/tests/system/Log/Handlers/FileHandlerTest.php b/tests/system/Log/Handlers/FileHandlerTest.php index ab16dee5615f..ddd1ca783f7c 100644 --- a/tests/system/Log/Handlers/FileHandlerTest.php +++ b/tests/system/Log/Handlers/FileHandlerTest.php @@ -114,4 +114,34 @@ public function testHandleDateTimeCorrectly(): void $expectedResult = 'Test message'; $this->assertStringContainsString($expectedResult, (string) $line); } + + public function testHandleReturnsFalseOnFailedWriteByDefault(): void + { + $logger = new FileHandler(['path' => $this->start . 'missing/']); + + $this->assertFalse($logger->handle('warning', 'This is a test log')); + } + + public function testHandleReturnsFalseOnFailedWriteWhenStopChainOnFailureIsEnabled(): void + { + $logger = new FileHandler(['path' => $this->start . 'missing/', 'stopChainOnFailure' => true]); + + $this->assertFalse($logger->handle('warning', 'This is a test log')); + } + + public function testHandleReturnsTrueOnFailedWriteWhenStopChainOnFailureIsDisabled(): void + { + $logger = new FileHandler(['path' => $this->start . 'missing/', 'stopChainOnFailure' => false]); + + $this->assertTrue($logger->handle('warning', 'This is a test log')); + $this->assertDirectoryDoesNotExist($this->start . 'missing/'); + } + + public function testHandleReturnsTrueOnSuccessfulWriteWhenStopChainOnFailureIsDisabled(): void + { + $logger = new FileHandler(['path' => $this->start, 'stopChainOnFailure' => false]); + + $this->assertTrue($logger->handle('warning', 'This is a test log')); + $this->assertFileExists($this->start . 'log-' . date('Y-m-d') . '.log'); + } } diff --git a/tests/system/Log/LoggerTest.php b/tests/system/Log/LoggerTest.php index 487a4d4b9b9e..b8ee192f2b07 100644 --- a/tests/system/Log/LoggerTest.php +++ b/tests/system/Log/LoggerTest.php @@ -18,8 +18,10 @@ use CodeIgniter\Exceptions\RuntimeException; use CodeIgniter\I18n\Time; use CodeIgniter\Log\Exceptions\LogException; +use CodeIgniter\Log\Handlers\FileHandler; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockLogger as LoggerConfig; +use org\bovigo\vfs\vfsStream; use PHPUnit\Framework\Attributes\Group; use ReflectionMethod; use ReflectionNamedType; @@ -43,6 +45,54 @@ protected function tearDown(): void service('context')->clearAll(); // Clear any context data that may have been set during tests. } + public function testLogStopsRemainingHandlersWhenFileHandlerFailsByDefault(): void + { + $logger = new Logger($this->getConfigWithFailingFileHandler()); + + $logger->log('error', 'Test message'); + + $this->assertSame([], TestHandler::getLogs()); + } + + public function testLogRunsRemainingHandlersWhenFileHandlerFailsWithoutStoppingChain(): void + { + $logger = new Logger($this->getConfigWithFailingFileHandler(['stopChainOnFailure' => false])); + + $logger->log('error', 'Test message'); + + $logs = TestHandler::getLogs(); + + $this->assertCount(1, $logs); + $this->assertStringContainsString('Test message', $logs[0]); + } + + /** + * Returns a config with a FileHandler pointed at a missing directory, + * followed by the TestHandler. + * + * @param array{stopChainOnFailure?: bool} $fileHandlerConfig + */ + private function getConfigWithFailingFileHandler(array $fileHandlerConfig = []): LoggerConfig + { + $config = new LoggerConfig(); + $testHandlerConfig = $config->handlers[TestHandler::class]; + + // The TestHandler only clears its stored logs when it is instantiated, + // which does not happen when an earlier handler stops the chain. + new TestHandler($testHandlerConfig); + + $config->handlers = [ + FileHandler::class => [ + 'handles' => ['error'], + 'path' => vfsStream::setup('root')->url() . '/missing/', + ...$fileHandlerConfig, + ], + TestHandler::class => $testHandlerConfig, + ]; + + return $config; + } + public function testThrowsExceptionWithBadHandlerSettings(): void { $config = new LoggerConfig(); diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index ae6a171fcfa1..96da5854b045 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -332,6 +332,7 @@ Libraries - **Locks:** Added :doc:`Atomic Locks ` for owner-aware, cross-process mutual exclusion backed by supported cache handlers: **File**, **Redis**, **Predis**, and **Memcached**. Memcached support has driver-specific release limitations because Memcached has no atomic compare-and-delete command. - **Logging:** Log handlers now receive the full context array as a third argument to ``handle()``. When ``$logGlobalContext`` is enabled, the CI global context is available under the ``HandlerInterface::GLOBAL_CONTEXT_KEY`` key. Built-in handlers append it to the log output; custom handlers can use it for structured logging. - **Logging:** Added :ref:`per-call context logging ` with three new ``Config\Logger`` options (``$logContext``, ``$logContextTrace``, ``$logContextUsedKeys``). Per PSR-3, a ``Throwable`` in the ``exception`` context key is automatically normalized to a meaningful array. All options default to ``false``. +- **Logging:** Added the ``stopChainOnFailure`` handler option for ``FileHandler`` and ``ErrorlogHandler``. Set it to ``false`` to let the remaining handlers run when the handler fails to write the log. It defaults to ``true``, which keeps the existing behavior. See :ref:`logging-stop-chain-on-failure`. - **Security:** Added :ref:`Fetch Metadata based CSRF protection ` with token fallback. Helpers and Functions diff --git a/user_guide_src/source/general/logging.rst b/user_guide_src/source/general/logging.rst index 6d241951adf7..7c2df5dfb63d 100644 --- a/user_guide_src/source/general/logging.rst +++ b/user_guide_src/source/general/logging.rst @@ -81,6 +81,31 @@ Each handler's section will have one property in common: ``handles``, which is a .. literalinclude:: logging/004.php +.. _logging-stop-chain-on-failure: + +Stopping the Handler Chain on Failure +------------------------------------- + +.. versionadded:: 4.8.0 + +Handlers are executed in the order they are defined in the ``$handlers`` property. When a handler's +``handle()`` method returns ``false``, the handlers defined after it are not executed. + +By default, the **File Handler** and the **Errorlog Handler** return ``false`` when they fail to write the +log, for example when the log directory is not writable. This means that a failing handler prevents the +handlers after it from logging the message. + +If you want the remaining handlers to run even when a handler fails to write the log, set its +``stopChainOnFailure`` option to ``false``: + +.. literalinclude:: logging/010.php + +In the example above, the **Errorlog Handler** still logs the message when the **File Handler** cannot write +to the log file. + +.. note:: This option only affects failed writes in the **File Handler** and the **Errorlog Handler**. + A custom handler can still return ``false`` from ``handle()`` to stop the remaining handlers. + Modifying the Message with Context ================================== diff --git a/user_guide_src/source/general/logging/010.php b/user_guide_src/source/general/logging/010.php new file mode 100644 index 000000000000..ed7319ea06c3 --- /dev/null +++ b/user_guide_src/source/general/logging/010.php @@ -0,0 +1,22 @@ + [ + 'handles' => ['critical', 'alert', 'emergency', 'debug', 'error', 'info', 'notice', 'warning'], + 'stopChainOnFailure' => false, + ], + // Errorlog Handler + 'CodeIgniter\Log\Handlers\ErrorlogHandler' => [ + 'handles' => ['critical', 'alert', 'emergency', 'debug', 'error', 'info', 'notice', 'warning'], + ], + ]; + + // ... +} diff --git a/user_guide_src/source/installation/upgrade_480.rst b/user_guide_src/source/installation/upgrade_480.rst index 7aebe713dc65..c4078f942f23 100644 --- a/user_guide_src/source/installation/upgrade_480.rst +++ b/user_guide_src/source/installation/upgrade_480.rst @@ -169,6 +169,8 @@ Config - Added a new filter named ``requestid`` that adds a unique request ID to each request in the application's context. - app/Config/Generators.php - ``Config\Generators::$views`` added entries for ``make:request``, ``make:test``, and ``make:transformer``, and dropped the stale ``session:migration`` entry. +- app/Config/Logger.php + - ``Config\Logger::$handlers`` added a new key ``stopChainOnFailure`` to the ``FileHandler`` and ``ErrorlogHandler`` settings. - app/Config/Mimes.php - ``Config\Mimes::$mimes`` added a new key ``md`` for Markdown files. - app/Config/Routing.php