Browse Source

Merge pull request #15011 from cakephp/only-http-codes

Set default error code to 500 for HttpException only
othercorey 5 years ago
parent
commit
589aa920bb

+ 0 - 5
src/Controller/Exception/MissingActionException.php

@@ -26,9 +26,4 @@ class MissingActionException extends Exception
      * @inheritDoc
      */
     protected $_messageTemplate = 'Action %s::%s() could not be found, or is not accessible.';
-
-    /**
-     * @inheritDoc
-     */
-    protected $_defaultCode = 404;
 }

+ 1 - 1
src/Core/Exception/Exception.php

@@ -51,7 +51,7 @@ class Exception extends RuntimeException
      *
      * @var int
      */
-    protected $_defaultCode = 500;
+    protected $_defaultCode = 0;
 
     /**
      * Constructor.

+ 0 - 5
src/Datasource/Exception/PageOutOfBoundsException.php

@@ -25,9 +25,4 @@ class PageOutOfBoundsException extends Exception
      * @inheritDoc
      */
     protected $_messageTemplate = 'Page number %s could not be found.';
-
-    /**
-     * @inheritDoc
-     */
-    protected $_defaultCode = 404;
 }

+ 0 - 4
src/Datasource/Exception/RecordNotFoundException.php

@@ -23,8 +23,4 @@ use Cake\Core\Exception\Exception;
  */
 class RecordNotFoundException extends Exception
 {
-    /**
-     * @inheritDoc
-     */
-    protected $_defaultCode = 404;
 }

+ 8 - 3
src/Error/ConsoleErrorHandler.php

@@ -16,7 +16,9 @@ declare(strict_types=1);
  */
 namespace Cake\Error;
 
+use Cake\Command\Command;
 use Cake\Console\ConsoleOutput;
+use Cake\Console\Exception\ConsoleException;
 use Throwable;
 
 /**
@@ -61,9 +63,12 @@ class ConsoleErrorHandler extends BaseErrorHandler
     {
         $this->_displayException($exception);
         $this->logException($exception);
-        $code = $exception->getCode();
-        $code = $code && is_int($code) ? $code : 1;
-        $this->_stop($code);
+
+        $exitCode = Command::CODE_ERROR;
+        if ($exception instanceof ConsoleException) {
+            $exitCode = (int)$exception->getCode();
+        }
+        $this->_stop($exitCode);
     }
 
     /**

+ 4 - 0
src/Http/Exception/HttpException.php

@@ -26,4 +26,8 @@ use Cake\Core\Exception\Exception;
  */
 class HttpException extends Exception
 {
+    /**
+     * @inheritDoc
+     */
+    protected $_defaultCode = 500;
 }

+ 0 - 4
src/Network/Exception/SocketException.php

@@ -22,8 +22,4 @@ use Cake\Core\Exception\Exception;
  */
 class SocketException extends Exception
 {
-    /**
-     * @inheritDoc
-     */
-    protected $_defaultCode = 0;
 }

+ 0 - 4
src/Utility/Exception/XmlException.php

@@ -22,8 +22,4 @@ use Cake\Core\Exception\Exception;
  */
 class XmlException extends Exception
 {
-    /**
-     * @inheritDoc
-     */
-    protected $_defaultCode = 0;
 }

+ 12 - 40
tests/TestCase/Error/ConsoleErrorHandlerTest.php

@@ -16,10 +16,8 @@ declare(strict_types=1);
  */
 namespace Cake\Test\TestCase\Error;
 
+use Cake\Console\Exception\ConsoleException;
 use Cake\Controller\Exception\MissingActionException;
-use Cake\Core\Exception\Exception;
-use Cake\Http\Exception\InternalErrorException;
-use Cake\Http\Exception\NotFoundException;
 use Cake\Log\Log;
 use Cake\TestSuite\Stub\ConsoleOutput;
 use Cake\TestSuite\TestCase;
@@ -109,7 +107,8 @@ class ConsoleErrorHandlerTest extends TestCase
         $message = sprintf("<error>Exception:</error> Missing action\nIn [%s, line %s]\n", $exception->getFile(), $exception->getLine());
 
         $this->Error->expects($this->once())
-            ->method('_stop');
+            ->method('_stop')
+            ->with(1);
 
         $this->Error->handleException($exception);
 
@@ -126,55 +125,28 @@ class ConsoleErrorHandlerTest extends TestCase
     {
         $exception = new \InvalidArgumentException('Too many parameters.');
 
-        $this->Error->handleException($exception);
-        $this->assertStringContainsString('Too many parameters', $this->stderr->messages()[0]);
-    }
-
-    /**
-     * test a Error404 exception.
-     *
-     * @return void
-     */
-    public function testError404Exception()
-    {
-        $exception = new NotFoundException("don't use me in cli.");
-
-        $this->Error->handleException($exception);
-        $this->assertStringContainsString("don't use me in cli", $this->stderr->messages()[0]);
-    }
-
-    /**
-     * test a Error500 exception.
-     *
-     * @return void
-     */
-    public function testError500Exception()
-    {
-        $exception = new InternalErrorException("don't use me in cli.");
+        $this->Error->expects($this->once())
+            ->method('_stop')
+            ->with(1);
 
         $this->Error->handleException($exception);
-        $this->assertStringContainsString("don't use me in cli", $this->stderr->messages()[0]);
+        $this->assertStringContainsString('Too many parameters', $this->stderr->messages()[0]);
     }
 
     /**
-     * test a exception with non-integer code
+     * Test error code is used as exit code for ConsoleException.
      *
      * @return void
      */
-    public function testNonIntegerExceptionCode()
+    public function testConsoleExceptions()
     {
-        $exception = new Exception('Non-integer exception code');
-
-        $class = new \ReflectionClass('Exception');
-        $property = $class->getProperty('code');
-        $property->setAccessible(true);
-        $property->setValue($exception, '42S22');
+        $exception = new ConsoleException('Test ConsoleException', 2);
 
         $this->Error->expects($this->once())
             ->method('_stop')
-            ->with(1);
+            ->with(2);
 
         $this->Error->handleException($exception);
-        $this->assertStringContainsString('Non-integer exception code', $this->stderr->messages()[0]);
+        $this->assertStringContainsString('Test ConsoleException', $this->stderr->messages()[0]);
     }
 }

+ 27 - 39
tests/TestCase/ExceptionsTest.php

@@ -69,13 +69,6 @@ class ExceptionsTest extends TestCase
         $this->assertSame(__FILE__, $exception->getFile());
         $this->assertSame(1, $exception->getLine());
         $this->assertSame($previous, $exception->getPrevious());
-
-        $exception = new FatalErrorException('message', null, __FILE__, 1, $previous);
-        $this->assertSame('message', $exception->getMessage());
-        $this->assertSame(500, $exception->getCode());
-        $this->assertSame(__FILE__, $exception->getFile());
-        $this->assertSame(1, $exception->getLine());
-        $this->assertSame($previous, $exception->getPrevious());
     }
 
     /**
@@ -93,11 +86,6 @@ class ExceptionsTest extends TestCase
         $this->assertSame(100, $exception->getCode());
         $this->assertSame($previous, $exception->getPrevious());
         $this->assertSame($entity, $exception->getEntity());
-
-        $exception = new PersistenceFailedException(new Entity(), 'message', null, $previous);
-        $this->assertSame('message', $exception->getMessage());
-        $this->assertSame(500, $exception->getCode());
-        $this->assertSame($previous, $exception->getPrevious());
     }
 
     /**
@@ -156,24 +144,24 @@ class ExceptionsTest extends TestCase
             ['Cake\Console\Exception\MissingTaskException', 1],
             ['Cake\Console\Exception\StopException', 1],
             ['Cake\Controller\Exception\AuthSecurityException', 400],
-            ['Cake\Controller\Exception\MissingActionException', 404],
-            ['Cake\Controller\Exception\MissingComponentException', 500],
+            ['Cake\Controller\Exception\MissingActionException', 0],
+            ['Cake\Controller\Exception\MissingComponentException', 0],
             ['Cake\Controller\Exception\SecurityException', 400],
-            ['Cake\Core\Exception\Exception', 500],
-            ['Cake\Core\Exception\MissingPluginException', 500],
-            ['Cake\Database\Exception', 500],
-            ['Cake\Database\Exception\MissingConnectionException', 500],
-            ['Cake\Database\Exception\MissingDriverException', 500],
-            ['Cake\Database\Exception\MissingExtensionException', 500],
-            ['Cake\Database\Exception\NestedTransactionRollbackException', 500],
-            ['Cake\Datasource\Exception\InvalidPrimaryKeyException', 500],
-            ['Cake\Datasource\Exception\MissingDatasourceConfigException', 500],
-            ['Cake\Datasource\Exception\MissingDatasourceException', 500],
-            ['Cake\Datasource\Exception\MissingModelException', 500],
-            ['Cake\Datasource\Exception\PageOutOfBoundsException', 404],
-            ['Cake\Datasource\Exception\RecordNotFoundException', 404],
-            ['Cake\Mailer\Exception\MissingActionException', 500],
-            ['Cake\Mailer\Exception\MissingMailerException', 500],
+            ['Cake\Core\Exception\Exception', 0],
+            ['Cake\Core\Exception\MissingPluginException', 0],
+            ['Cake\Database\Exception', 0],
+            ['Cake\Database\Exception\MissingConnectionException', 0],
+            ['Cake\Database\Exception\MissingDriverException', 0],
+            ['Cake\Database\Exception\MissingExtensionException', 0],
+            ['Cake\Database\Exception\NestedTransactionRollbackException', 0],
+            ['Cake\Datasource\Exception\InvalidPrimaryKeyException', 0],
+            ['Cake\Datasource\Exception\MissingDatasourceConfigException', 0],
+            ['Cake\Datasource\Exception\MissingDatasourceException', 0],
+            ['Cake\Datasource\Exception\MissingModelException', 0],
+            ['Cake\Datasource\Exception\PageOutOfBoundsException', 0],
+            ['Cake\Datasource\Exception\RecordNotFoundException', 0],
+            ['Cake\Mailer\Exception\MissingActionException', 0],
+            ['Cake\Mailer\Exception\MissingMailerException', 0],
             ['Cake\Http\Exception\BadRequestException', 400],
             ['Cake\Http\Exception\ConflictException', 409],
             ['Cake\Http\Exception\ForbiddenException', 403],
@@ -190,18 +178,18 @@ class ExceptionsTest extends TestCase
             ['Cake\Http\Exception\UnauthorizedException', 401],
             ['Cake\Http\Exception\UnavailableForLegalReasonsException', 451],
             ['Cake\Network\Exception\SocketException', 0],
-            ['Cake\ORM\Exception\MissingBehaviorException', 500],
-            ['Cake\ORM\Exception\MissingEntityException', 500],
-            ['Cake\ORM\Exception\MissingTableClassException', 500],
-            ['Cake\ORM\Exception\RolledbackTransactionException', 500],
-            ['Cake\Routing\Exception\DuplicateNamedRouteException', 500],
-            ['Cake\Routing\Exception\MissingDispatcherFilterException', 500],
-            ['Cake\Routing\Exception\MissingRouteException', 500],
+            ['Cake\ORM\Exception\MissingBehaviorException', 0],
+            ['Cake\ORM\Exception\MissingEntityException', 0],
+            ['Cake\ORM\Exception\MissingTableClassException', 0],
+            ['Cake\ORM\Exception\RolledbackTransactionException', 0],
+            ['Cake\Routing\Exception\DuplicateNamedRouteException', 0],
+            ['Cake\Routing\Exception\MissingDispatcherFilterException', 0],
+            ['Cake\Routing\Exception\MissingRouteException', 0],
             ['Cake\Routing\Exception\RedirectException', 302],
             ['Cake\Utility\Exception\XmlException', 0],
-            ['Cake\View\Exception\MissingCellException', 500],
-            ['Cake\View\Exception\MissingHelperException', 500],
-            ['Cake\View\Exception\MissingViewException', 500],
+            ['Cake\View\Exception\MissingCellException', 0],
+            ['Cake\View\Exception\MissingHelperException', 0],
+            ['Cake\View\Exception\MissingViewException', 0],
         ];
     }
 }