Merge pull request #16495 from cakephp/5.x-error-deprecated

Removed deprecated code for error handling.

src/Error/ErrorLogger.php

@@ -53,12 +53,7 @@ class ErrorLogger implements ErrorLoggerInterface
     }
 
     /**
-     * Log an error to Cake's Log subsystem
-     *
-     * @param \Cake\Error\PhpError $error The error to log
-     * @param ?\Psr\Http\Message\ServerRequestInterface $request The request if in an HTTP context.
-     * @param bool $includeTrace Should the log message include a stacktrace
-     * @return void
+     * @inheritDoc
      */
     public function logError(PhpError $error, ?ServerRequestInterface $request = null, bool $includeTrace = false): void
     {
@@ -80,12 +75,7 @@ class ErrorLogger implements ErrorLoggerInterface
     }
 
     /**
-     * Log an exception to Cake's Log subsystem
-     *
-     * @param \Throwable $exception The exception to log a message for.
-     * @param \Psr\Http\Message\ServerRequestInterface|null $request The current request if available.
-     * @param bool $includeTrace Whether or not a stack trace should be logged.
-     * @return void
+     * @inheritDoc
      */
     public function logException(
         Throwable $exception,
@@ -101,49 +91,6 @@ class ErrorLogger implements ErrorLoggerInterface
     }
 
     /**
-     * @param string|int $level The logging level
-     * @param string $message The message to be logged.
-     * @param array $context Context.
-     * @return bool
-     * @deprecated 4.4.0 Use logError instead.
-     */
-    public function logMessage($level, string $message, array $context = []): bool
-    {
-        if (!empty($context['request'])) {
-            $message .= $this->getRequestContext($context['request']);
-        }
-        if (!empty($context['trace'])) {
-            $message .= "\nTrace:\n" . $context['trace'] . "\n";
-        }
-        $logMap = [
-            'strict' => LOG_NOTICE,
-            'deprecated' => LOG_NOTICE,
-        ];
-        $level = $logMap[$level] ?? $level;
-
-        return Log::write($level, $message);
-    }
-
-    /**
-     * @param \Throwable $exception The exception to log a message for.
-     * @param \Psr\Http\Message\ServerRequestInterface|null $request The current request if available.
-     * @return bool
-     * @deprecated 4.4.0 Use logException instead.
-     */
-    public function log(Throwable $exception, ?ServerRequestInterface $request = null): bool
-    {
-        $message = $this->getMessage($exception, false, $this->getConfig('trace'));
-
-        if ($request !== null) {
-            $message .= $this->getRequestContext($request);
-        }
-
-        $message .= "\n\n";
-
-        return Log::error($message);
-    }
-
-    /**
      * Generate the message for the exception
      *
      * @param \Throwable $exception The exception to log a message for.

src/Error/ErrorLoggerInterface.php

@@ -22,13 +22,7 @@ use Throwable;
 /**
  * Interface for error logging handlers.
  *
- * Used by the ErrorHandlerMiddleware and global
- * error handlers to log exceptions and errors.
- *
- * @method void logException(\Throwable $exception, ?\Psr\Http\Message\ServerRequestInterface $request = null, bool $includeTrace = false)
- *   Log an exception with an optional HTTP request.
- * @method void logError(\Cake\Error\PhpError $error, ?\Psr\Http\Message\ServerRequestInterface $request = null, bool $includeTrace = false)
- *   Log an error with an optional HTTP request.
+ * Used by the ErrorHandlerMiddleware and global error handlers to log exceptions and errors.
  */
 interface ErrorLoggerInterface
 {
@@ -37,22 +31,26 @@ interface ErrorLoggerInterface
      *
      * @param \Throwable $exception The exception to log a message for.
      * @param \Psr\Http\Message\ServerRequestInterface|null $request The current request if available.
-     * @return bool
-     * @deprecated 4.4.0 Implement `logException` instead.
+     * @param bool $includeTrace Should the log message include a stacktrace.
+     * @return void
      */
-    public function log(
+    public function logException(
         Throwable $exception,
-        ?ServerRequestInterface $request = null
-    ): bool;
+        ?ServerRequestInterface $request = null,
+        bool $includeTrace = false
+    ): void;
 
     /**
-     * Log a an error message to the error logger.
+     * Log an error to Cake's Log subsystem
      *
-     * @param string|int $level The logging level
-     * @param string $message The message to be logged.
-     * @param array $context Context.
-     * @return bool
-     * @deprecated 4.4.0 Implement `logError` instead.
+     * @param \Cake\Error\PhpError $error The error to log.
+     * @param \Psr\Http\Message\ServerRequestInterface|null $request The request if in an HTTP context.
+     * @param bool $includeTrace Should the log message include a stacktrace.
+     * @return void
      */
-    public function logMessage(string|int $level, string $message, array $context = []): bool;
+    public function logError(
+        PhpError $error,
+        ?ServerRequestInterface $request = null,
+        bool $includeTrace = false
+    ): void;
 }

src/Error/ErrorTrap.php

@@ -138,7 +138,7 @@ class ErrorTrap
             $renderer->write($renderer->render($error, $debug));
         } catch (Exception $e) {
             // Fatal errors always log.
-            $this->logger()->logMessage('error', 'Could not render error. Got: ' . $e->getMessage());
+            $this->logger()->logException($e);
 
             return false;
         }
@@ -157,24 +157,7 @@ class ErrorTrap
         if (!$this->_config['log']) {
             return;
         }
-        $logger = $this->logger();
-        if (method_exists($logger, 'logError')) {
-            $logger->logError($error, Router::getRequest(), $this->_config['trace']);
-        } else {
-            $loggerClass = get_class($logger);
-            deprecationWarning(
-                "The configured logger `{$loggerClass}` does not implement `logError()` " .
-                'which will be required in future versions of CakePHP.'
-            );
-            $context = [];
-            if ($this->_config['trace']) {
-                $context = [
-                    'trace' => $error->getTraceAsString(),
-                    'request' => Router::getRequest(),
-                ];
-            }
-            $logger->logMessage($error->getLabel(), $error->getMessage(), $context);
-        }
+        $this->logger()->logError($error, Router::getRequest(), $this->_config['trace']);
     }
 
     /**

src/Error/ExceptionTrap.php

@@ -341,17 +341,7 @@ class ExceptionTrap
             }
         }
         if ($shouldLog) {
-            $logger = $this->logger();
-            if (method_exists($logger, 'logException')) {
-                $logger->logException($exception, $request, $this->_config['trace']);
-            } else {
-                $loggerClass = get_class($logger);
-                deprecationWarning(
-                    "The configured logger `{$loggerClass}` should implement `logException()` " .
-                    'to be compatible with future versions of CakePHP.'
-                );
-                $this->logger()->log($exception, $request);
-            }
+            $this->logger()->logException($exception, $request, $this->_config['trace']);
         }
         $this->dispatchEvent('Exception.beforeRender', ['exception' => $exception]);
     }

src/Error/Renderer/ConsoleErrorRenderer.php

@@ -35,7 +35,7 @@ class ConsoleErrorRenderer implements ErrorRendererInterface
     /**
      * @var bool
      */
-    protected $trace = false;
+    protected bool $trace = false;
 
     /**
      * Constructor.

tests/TestCase/Error/ErrorTrapTest.php

@@ -24,14 +24,12 @@ use Cake\Error\PhpError;
 use Cake\Error\Renderer\ConsoleErrorRenderer;
 use Cake\Error\Renderer\HtmlErrorRenderer;
 use Cake\Error\Renderer\TextErrorRenderer;
-use Cake\Http\ServerRequest;
 use Cake\Log\Log;
 use Cake\Routing\Router;
 use Cake\TestSuite\Stub\ConsoleOutput;
 use Cake\TestSuite\TestCase;
 use InvalidArgumentException;
 use stdClass;
-use TestApp\Error\LegacyErrorLogger;
 
 class ErrorTrapTest extends TestCase
 {
@@ -191,37 +189,6 @@ class ErrorTrapTest extends TestCase
         $this->assertEmpty($logs);
     }
 
-    public function testHandleErrorLogDeprecatedLoggerMethods()
-    {
-        $request = new ServerRequest([
-            'url' => '/articles/view/1',
-        ]);
-        Router::setRequest($request);
-
-        Log::setConfig('test_error', [
-            'className' => 'Array',
-        ]);
-        $trap = new ErrorTrap([
-            'errorRenderer' => TextErrorRenderer::class,
-            'logger' => LegacyErrorLogger::class,
-            'log' => true,
-            'trace' => true,
-        ]);
-
-        $this->deprecated(function () use ($trap) {
-            // Calling this method directly as deprecated() interferes with registering
-            // an error handler.
-            ob_start();
-            $trap->handleError(E_USER_WARNING, 'Oh no it was bad', __FILE__, __LINE__);
-            ob_get_clean();
-        });
-
-        $logs = Log::engine('test_error')->read();
-        $this->assertStringContainsString('Oh no it was bad', $logs[0]);
-        $this->assertStringContainsString('IncludeTrace', $logs[0]);
-        $this->assertStringContainsString('URL=http://localhost/articles/view/1', $logs[0]);
-    }
-
     public function testConsoleRenderingNoTrace()
     {
         $stub = new ConsoleOutput();

tests/TestCase/Error/ExceptionTrapTest.php

@@ -22,14 +22,12 @@ use Cake\Error\Renderer\ConsoleExceptionRenderer;
 use Cake\Error\Renderer\TextExceptionRenderer;
 use Cake\Error\Renderer\WebExceptionRenderer;
 use Cake\Http\Exception\MissingControllerException;
-use Cake\Http\ServerRequest;
 use Cake\Log\Log;
 use Cake\TestSuite\Stub\ConsoleOutput;
 use Cake\TestSuite\TestCase;
 use Cake\Utility\Text;
 use InvalidArgumentException;
 use stdClass;
-use TestApp\Error\LegacyErrorLogger;
 use Throwable;
 
 class ExceptionTrapTest extends TestCase
@@ -232,28 +230,6 @@ class ExceptionTrapTest extends TestCase
         $this->assertEmpty($logs);
     }
 
-    public function testLogExceptionDeprecatedLoggerMethods()
-    {
-        Log::setConfig('test_error', [
-            'className' => 'Array',
-        ]);
-        $trap = new ExceptionTrap([
-            'log' => true,
-            'logger' => LegacyErrorLogger::class,
-            'trace' => true,
-        ]);
-        $error = new InvalidArgumentException('nope');
-        $request = new ServerRequest(['url' => '/articles/view/1']);
-        $this->deprecated(function () use ($trap, $error, $request) {
-            $trap->logException($error, $request);
-        });
-
-        $logs = Log::engine('test_error')->read();
-        $this->assertStringContainsString('nope', $logs[0]);
-        $this->assertStringContainsString('IncludeTrace', $logs[0]);
-        $this->assertStringContainsString('URL=http://localhost/articles/view/1', $logs[0]);
-    }
-
     /**
      * @preserveGlobalState disabled
      * @runInSeparateProcess

tests/test_app/TestApp/Error/LegacyErrorLogger.php

@@ -1,94 +0,0 @@
-<?php
-declare(strict_types=1);
-
-/**
- * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
- * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- *
- * Licensed under The MIT License
- * For full copyright and license information, please see the LICENSE.txt
- * Redistributions of files must retain the above copyright notice.
- *
- * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
- * @link          https://cakephp.org CakePHP(tm) Project
- * @since         4.4.0
- * @license       https://opensource.org/licenses/mit-license.php MIT License
- */
-namespace TestApp\Error;
-
-use Cake\Core\InstanceConfigTrait;
-use Cake\Error\ErrorLoggerInterface;
-use Cake\Log\Log;
-use Psr\Http\Message\ServerRequestInterface;
-use Throwable;
-
-/**
- * Test stub that only implements the interface as declared.
- * Necessary to test the deprecated logging paths in ErrorTrap and ExceptionTrap
- */
-class LegacyErrorLogger implements ErrorLoggerInterface
-{
-    use InstanceConfigTrait;
-
-    /**
-     * Default configuration values.
-     *
-     * - `trace` Should error logs include stack traces?
-     *
-     * @var array<string, mixed>
-     */
-    protected $_defaultConfig = [
-        'trace' => false,
-    ];
-
-    /**
-     * Constructor
-     *
-     * @param array<string, mixed> $config Config array.
-     */
-    public function __construct(array $config = [])
-    {
-        $this->setConfig($config);
-    }
-
-    /**
-     * @param string|int $level The logging level
-     * @param string $message The message to be logged.
-     * @param array $context Context.
-     * @return bool
-     */
-    public function logMessage($level, string $message, array $context = []): bool
-    {
-        if (!empty($context['request'])) {
-            $message .= ' URL=' . $context['request']->getUri();
-        }
-        if (!empty($context['trace'])) {
-            $message .= ' IncludeTrace';
-        }
-        $logMap = [
-            'strict' => LOG_NOTICE,
-            'deprecated' => LOG_NOTICE,
-        ];
-        $level = $logMap[$level] ?? $level;
-
-        return Log::write($level, $message);
-    }
-
-    /**
-     * @param \Throwable $exception The exception to log a message for.
-     * @param \Psr\Http\Message\ServerRequestInterface|null $request The current request if available.
-     * @return bool
-     */
-    public function log(Throwable $exception, ?ServerRequestInterface $request = null): bool
-    {
-        $message = $exception->getMessage();
-        if ($request !== null) {
-            $message .= 'URL=' . $request->getUri();
-        }
-        if ($this->getConfig('trace')) {
-            $message .= 'IncludeTrace';
-        }
-
-        return Log::error($message);
-    }
-}