Fix Error logging for 404s.

.travis.yml

@@ -52,7 +52,7 @@ script:
 
   - if [[ $CODECOVERAGE == 1 ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=clover.xml; fi
 
-  - if [[ $CHECKS == 1 ]]; then composer phpstan-setup && composer phpstan ; fi
+  - if [[ $CHECKS == 1 ]]; then composer stan-setup && composer stan ; fi
   - if [[ $CHECKS == 1 ]]; then composer cs-check ; fi
 
 after_success:

composer.json

@@ -45,8 +45,8 @@
 		"issues": "https://github.com/dereuromark/cakephp-tools/issues"
 	},
 	"scripts": {
-		"phpstan": "phpstan analyse -c tests/phpstan.neon -l 5 src/",
-		"phpstan-setup": "cp composer.json composer.backup && composer require --dev phpstan/phpstan:^0.12.1 && mv composer.backup composer.json",
+		"stan": "phpstan analyse -c tests/phpstan.neon -l 5 src/",
+		"stan-setup": "cp composer.json composer.backup && composer require --dev phpstan/phpstan:^0.12.1 && mv composer.backup composer.json",
 		"test": "php phpunit.phar",
 		"test-setup": "[ ! -f phpunit.phar ] && wget https://phar.phpunit.de/phpunit-8.5.1.phar && mv phpunit-8.5.1.phar phpunit.phar || true",
 		"test-coverage": "php phpunit.phar --log-junit webroot/coverage/unitreport.xml --coverage-html webroot/coverage --coverage-clover webroot/coverage/coverage.xml",

config/app.default.php

@@ -4,6 +4,21 @@ $config = [
 	'Paginator' => [
 	],
 
+	// Error handling around 404s
+	'Log' => [
+		'debug' => [
+			'scopes' => false,
+		],
+		'error' => [
+			'scopes' => false,
+		],
+		'404' => [
+			'file' => '404',
+			'levels' => ['error'],
+			'scopes' => ['404'],
+		],
+	],
+
 	// Controller pagination
 	'DataPreparation' => [
 		'noTrim' => false,

docs/Error/ErrorHandler.md

@@ -3,12 +3,12 @@
 The main goal of the error.log is to notify about internal errors of the system.
 By default there would also be a lot of noise in there.
 
-Most 404 logs should not be part of your error log, for example. 
+Most 404 logs should not be part of your error log, for example.
 You can either completely ignore them, or better yet put them into their own space:
 ```php
 Log::config('404', [
     'className' => '...', // e.g. 'File' or 'DatabaseLog.Database'
-    'type' => '404',
+    'file' => '404',
     'levels' => ['error'],
     'scopes' => ['404'],
 ]);
@@ -53,7 +53,7 @@ class Application extends BaseApplication {
      *
      * @return \Cake\Http\MiddlewareQueue The updated middleware queue.
      */
-    public function middleware($middlewareQueue) {
+    public function middleware(MiddlewareQueue $middlewareQueue): MiddlewareQueue {
         $middlewareQueue
             // Replace the core one
             ->add(new ErrorHandlerMiddleware())

resources/locales/cs/tools.po

@@ -104,7 +104,7 @@ msgid "Delay necessary for address '{0}'"
 msgstr ""
 
 #: Lib/GeocodeLib.php:299;380
-msgid "Could not geocode '{0}'"
+msgid "Could not geocode ''{0}''"
 msgstr ""
 
 #: Lib/GeocodeLib.php:306;386

resources/locales/de/tools.po

@@ -104,7 +104,7 @@ msgid "Delay necessary for address '{0}'"
 msgstr ""
 
 #: Lib/GeocodeLib.php:299;380
-msgid "Could not geocode '{0}'"
+msgid "Could not geocode ''{0}''"
 msgstr ""
 
 #: Lib/GeocodeLib.php:306;386

resources/locales/en/tools.po

@@ -56,7 +56,7 @@ msgid "Delay necessary for address '{0}'"
 msgstr ""
 
 #: Lib/GeocodeLib.php:299;380
-msgid "Could not geocode '{0}'"
+msgid "Could not geocode ''{0}''"
 msgstr ""
 
 #: Lib/GeocodeLib.php:306;386

resources/locales/es/tools.po

@@ -104,7 +104,7 @@ msgid "Delay necessary for address '{0}'"
 msgstr ""
 
 #: Lib/GeocodeLib.php:299;380
-msgid "Could not geocode '{0}'"
+msgid "Could not geocode ''{0}''"
 msgstr ""
 
 #: Lib/GeocodeLib.php:306;386

src/Error/ErrorHandler.php

@@ -2,12 +2,7 @@
 
 namespace Tools\Error;
 
-use Cake\Error\Debugger;
 use Cake\Error\ErrorHandler as CoreErrorHandler;
-use Cake\Log\Log;
-use Cake\Routing\Router;
-use Psr\Http\Message\ServerRequestInterface;
-use Throwable;
 
 /**
  * Custom ErrorHandler to not mix the 404 exceptions with the rest of "real" errors in the error.log file.
@@ -28,63 +23,20 @@ use Throwable;
  *
  * In case you need custom 404 mappings for some additional custom exceptions, make use of `log404` option.
  * It will overwrite the current defaults completely.
- *
- * Somewhat deprecated of CakePHP 3.3+. Use Tools\Error\Middleware\ErrorHandlerMiddleware now.
- * Still needed for low level errors, though.
  */
 class ErrorHandler extends CoreErrorHandler {
 
-	use ErrorHandlerTrait;
-
-	/**
-	 * Handles exception logging
-	 *
-	 * @param \Throwable $exception Exception instance.
-	 * @param \Psr\Http\Message\ServerRequestInterface|null $request
-	 * @return bool
-	 */
-	public function logException(Throwable $exception, ?ServerRequestInterface $request = null): bool {
-		if ($this->is404($exception)) {
-			$level = LOG_ERR;
-			Log::write($level, $this->buildMessage($exception), ['404']);
-			return false;
-		}
-
-		return parent::logException($exception, $request ?? Router::getRequest());
-	}
-
 	/**
-	 * @param \Throwable $exception
+	 * Constructor
 	 *
-	 * @return string
+	 * @param array $config The options for error handling.
 	 */
-	protected function buildMessage(Throwable $exception): string {
-		$error = error_get_last();
-
-		$message = sprintf(
-			'%s (%s): %s in [%s, line %s]',
-			$exception->getMessage(),
-			$exception->getCode(),
-			$error['description'] ?? '',
-			$error['file'] ?? '',
-			$error['line'] ?? ''
-		);
-		if (!empty($this->_config['trace'])) {
-			/** @var string $trace */
-			$trace = Debugger::trace([
-				'start' => 1,
-				'format' => 'log',
-			]);
-
-			$request = Router::getRequest();
-			if ($request) {
-				$message .= $this->getLogger()->getRequestContext($request);
-			}
-			$message .= "\nTrace:\n" . $trace . "\n";
-		}
-		$message .= "\n\n";
+	public function __construct(array $config = []) {
+		$config += [
+			'errorLogger' => ErrorLogger::class,
+		];
 
-		return $message;
+		parent::__construct($config);
 	}
 
 }

src/Error/ErrorLogger.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace Tools\Error;
+
+use Cake\Error\ErrorLogger as CoreErrorLogger;
+use Cake\Log\Log;
+use Psr\Http\Message\ServerRequestInterface;
+use Throwable;
+
+class ErrorLogger extends CoreErrorLogger {
+
+	use ErrorHandlerTrait;
+
+	/**
+	 * Generate the error log 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 {
+		foreach ($this->getConfig('skipLog') as $class) {
+			if ($exception instanceof $class) {
+				return false;
+			}
+		}
+
+		if ($this->is404($exception)) {
+			$level = LOG_ERR;
+			$message = $this->getMessage($exception);
+
+			if ($request !== null) {
+				$message .= $this->getRequestContext($request);
+			}
+
+			$message .= "\n\n";
+
+			return Log::write($level, $message, ['404']);
+		}
+
+		return parent::log($exception, $request);
+	}
+
+}

src/Error/Middleware/ErrorHandlerMiddleware.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Tools\Error\Middleware;
+
+use Cake\Core\Configure;
+use Cake\Error\Middleware\ErrorHandlerMiddleware as CoreErrorHandlerMiddleware;
+use Tools\Error\ErrorHandler;
+
+/**
+ * Custom ErrorHandler to not mix the 404 exceptions with the rest of "real" errors in the error.log file.
+ */
+class ErrorHandlerMiddleware extends CoreErrorHandlerMiddleware {
+
+	/**
+	 * @param \Cake\Error\ErrorHandler|array $errorHandler The error handler instance
+	 *  or config array.
+	 */
+	public function __construct($errorHandler = []) {
+		if (is_array($errorHandler)) {
+			$errorHandler += Configure::read('Error');
+		}
+		parent::__construct($errorHandler);
+
+		$this->errorHandler = new ErrorHandler($this->getConfig());
+	}
+
+}

tests/TestCase/ErrorHandler/ErrorHandlerTest.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Tools\Test\TestCase\ErrorHandler;
+
+use Shim\TestSuite\TestCase;
+use Tools\Error\ErrorHandler;
+use Tools\Error\ErrorLogger;
+
+class ErrorHandlerTest extends TestCase {
+
+	/**
+	 * @var \Tools\Error\ErrorHandler
+	 */
+	protected $errorHandler;
+
+	/**
+	 * @return void
+	 */
+	public function setUp(): void {
+		parent::setUp();
+
+		$this->errorHandler = new ErrorHandler();
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testLogger(): void {
+		$result = $this->errorHandler->getConfig('errorLogger');
+		$this->assertSame(ErrorLogger::class, $result);
+	}
+
+}

tests/TestCase/ErrorHandler/ErrorLoggerTest.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace Tools\Test\TestCase\ErrorHandler;
+
+use Cake\Http\Exception\InternalErrorException;
+use Cake\Http\Exception\NotFoundException;
+use Shim\TestSuite\TestCase;
+use Shim\TestSuite\TestTrait;
+use Tools\Error\ErrorLogger;
+
+class ErrorLoggerTest extends TestCase {
+
+	use TestTrait;
+
+	/**
+	 * @var \Tools\Error\ErrorHandler
+	 */
+	protected $errorLogger;
+
+	/**
+	 * @return void
+	 */
+	public function setUp(): void {
+		parent::setUp();
+
+		$this->errorLogger = new ErrorLogger();
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testIs404(): void {
+		$exception = new NotFoundException();
+		$result = $this->invokeMethod($this->errorLogger, 'is404', [$exception]);
+
+		$this->assertTrue($result);
+
+		$exception = new InternalErrorException();
+		$result = $this->invokeMethod($this->errorLogger, 'is404', [$exception]);
+		$this->assertFalse($result);
+	}
+
+}

tests/TestCase/Form/ContactFormTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Form;
+namespace Tools\Test\TestCase\Form;
 
 use Shim\TestSuite\TestCase;
 

tests/TestCase/Model/Behavior/NeighborBehaviorTest.php

@@ -1,10 +1,9 @@
 <?php
 
-namespace Tools\Model\Behavior;
+namespace Tools\Test\TestCase\Model\Behavior;
 
 use Cake\ORM\TableRegistry;
 use Shim\TestSuite\TestCase;
-use Tools\Model\Table\Table;
 
 class NeighborBehaviorTest extends TestCase {
 

tests/TestCase/Model/Behavior/ResetBehaviorTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Model\Behavior;
+namespace Tools\Test\TestCase\Model\Behavior;
 
 use Cake\ORM\TableRegistry;
 use Shim\TestSuite\TestCase;

tests/TestCase/Model/Entity/EntityTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Model\Entity;
+namespace Tools\Test\TestCase\Model\Entity;
 
 use Cake\ORM\TableRegistry;
 use Shim\TestSuite\TestCase;

tests/TestCase/Model/Table/TableTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Model\Table;
+namespace Tools\Test\TestCase\Model\Table;
 
 use Cake\Datasource\ConnectionManager;
 use Cake\I18n\Time;

tests/TestCase/Model/Table/TokensTableTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Test\Model\Table;
+namespace Tools\Test\TestCase\Model\Table;
 
 use Cake\ORM\TableRegistry;
 use Shim\TestSuite\TestCase;

tests/TestCase/Utility/L10nTest.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Tools\Test\Utility;
+namespace Tools\Test\TestCase\Utility;
 
 use Shim\TestSuite\TestCase;
 use Tools\Utility\L10n;