Merge pull request #3344 from ADmad/3.0-exception

3.0 exception

src/Error/BaseException.php

@@ -1,50 +0,0 @@
-<?php
-/**
- * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
- * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
- *
- * Licensed under The MIT License
- * Redistributions of files must retain the above copyright notice.
- *
- * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
- * @link          http://book.cakephp.org/2.0/en/development/testing.html
- * @since         3.0.0
- * @license       MIT License (http://www.opensource.org/licenses/mit-license.php)
- */
-namespace Cake\Error;
-
-/**
- * Base class that all Exceptions extend.
- *
- */
-class BaseException extends \RuntimeException {
-
-/**
- * Array of headers to be passed to Cake\Network\Response::header()
- *
- * @var array
- */
-	protected $_responseHeaders = null;
-
-/**
- * Get/set the response header to be used
- *
- * See also Cake\Network\Response::header()
- *
- * @param string|array $header. An array of header strings or a single header string
- *	- an associative array of "header name" => "header value"
- *	- an array of string headers is also accepted
- * @param string $value The header value.
- * @return array
- */
-	public function responseHeader($header = null, $value = null) {
-		if ($header) {
-			if (is_array($header)) {
-				return $this->_responseHeaders = $header;
-			}
-			$this->_responseHeaders = array($header => $value);
-		}
-		return $this->_responseHeaders;
-	}
-
-}

src/Error/Exception.php

@@ -14,11 +14,10 @@
 namespace Cake\Error;
 
 /**
- * Exception is used a base class for CakePHP's internal exceptions.
- * In general framework errors are interpreted as 500 code errors.
+ * Base class that all CakePHP Exceptions extend.
  *
  */
-class Exception extends BaseException {
+class Exception extends \RuntimeException {
 
 /**
  * Array of attributes that are passed in from the constructor, and
@@ -36,6 +35,13 @@ class Exception extends BaseException {
 	protected $_messageTemplate = '';
 
 /**
+ * Array of headers to be passed to Cake\Network\Response::header()
+ *
+ * @var array
+ */
+	protected $_responseHeaders = null;
+
+/**
  * Constructor.
  *
  * Allows you to create exceptions that are treated as framework errors and disabled
@@ -62,4 +68,25 @@ class Exception extends BaseException {
 		return $this->_attributes;
 	}
 
+/**
+ * Get/set the response header to be used
+ *
+ * See also Cake\Network\Response::header()
+ *
+ * @param string|array $header. An array of header strings or a single header string
+ *	- an associative array of "header name" => "header value"
+ *	- an array of string headers is also accepted
+ * @param string $value The header value.
+ * @return array
+ */
+	public function responseHeader($header = null, $value = null) {
+		if ($header) {
+			if (is_array($header)) {
+				return $this->_responseHeaders = $header;
+			}
+			$this->_responseHeaders = array($header => $value);
+		}
+		return $this->_responseHeaders;
+	}
+
 }

src/Error/ExceptionRenderer.php

@@ -58,7 +58,7 @@ class ExceptionRenderer {
 	public $controller = null;
 
 /**
- * template to render for Cake\Error\Exception
+ * Template to render for Cake\Error\Exception
  *
  * @var string
  */
@@ -74,7 +74,7 @@ class ExceptionRenderer {
 /**
  * The exception being handled.
  *
- * @var Exception
+ * @var \Exception
  */
 	public $error = null;
 
@@ -87,39 +87,6 @@ class ExceptionRenderer {
  */
 	public function __construct(\Exception $exception) {
 		$this->controller = $this->_getController($exception);
-
-		list(, $baseClass) = namespaceSplit(get_class($exception));
-		$baseClass = substr($baseClass, 0, -9);
-		$method = $template = Inflector::variable($baseClass);
-		$code = $exception->getCode();
-
-		$methodExists = method_exists($this, $method);
-
-		if ($exception instanceof Exception && !$methodExists) {
-			$method = '_cakeError';
-			if (empty($template) || $template === 'internalError') {
-				$template = 'error500';
-			}
-		} elseif ($exception instanceof \PDOException) {
-			$method = 'pdoError';
-			$template = 'pdo_error';
-			$code = 500;
-		} elseif (!$methodExists) {
-			$method = 'error500';
-			if ($code >= 400 && $code < 500) {
-				$method = 'error400';
-			}
-		}
-
-		$isNotDebug = !Configure::read('debug');
-		if ($isNotDebug && $method === '_cakeError') {
-			$method = 'error400';
-		}
-		if ($isNotDebug && $code == 500) {
-			$method = 'error500';
-		}
-		$this->template = $template;
-		$this->method = $method;
 		$this->error = $exception;
 	}
 
@@ -129,7 +96,7 @@ class ExceptionRenderer {
  * This method returns the built in `ErrorController` normally, or if an error is repeated
  * a bare controller will be used.
  *
- * @param Exception $exception The exception to get a controller for.
+ * @param \Exception $exception The exception to get a controller for.
  * @return Controller
  */
 	protected function _getController($exception) {
@@ -138,10 +105,6 @@ class ExceptionRenderer {
 		}
 		$response = new Response();
 
-		if (method_exists($exception, 'responseHeader')) {
-			$response->header($exception->responseHeader());
-		}
-
 		try {
 			$controller = new ErrorController($request, $response);
 			$controller->startupProcess();
@@ -164,95 +127,126 @@ class ExceptionRenderer {
  * @return void
  */
 	public function render() {
-		if ($this->method) {
-			call_user_func_array(array($this, $this->method), array($this->error));
+		$exception = $this->error;
+		$code = $this->_code($exception);
+		$method = $this->_method($exception);
+		$template = $this->_template($exception, $method, $code);
+
+		$isDebug = Configure::read('debug');
+		if (($isDebug || $exception instanceof Error\HttpException) &&
+			method_exists($this, $method)
+		) {
+			call_user_func_array(array($this, $method), array($exception));
+			return;
 		}
-	}
 
-/**
- * Generic handler for the internal framework errors CakePHP can generate.
- *
- * @param \Cake\Error\Exception $error
- * @return void
- */
-	protected function _cakeError(Exception $error) {
+		$message = $this->_message($exception, $code);
 		$url = $this->controller->request->here();
-		$code = ($error->getCode() >= 400 && $error->getCode() < 506) ? $error->getCode() : 500;
+
+		if (method_exists($exception, 'responseHeader')) {
+			$this->controller->response->header($exception->responseHeader());
+		}
 		$this->controller->response->statusCode($code);
 		$this->controller->set(array(
-			'code' => $code,
-			'message' => h($error->getMessage()),
+			'message' => h($message),
 			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('code', 'message', 'url')
+			'error' => $exception,
+			'code' => $code,
+			'_serialize' => array('message', 'url', 'code')
 		));
-		$this->controller->set($error->getAttributes());
-		$this->_outputMessage($this->template);
+
+		if ($exception instanceof Error\Exception && $isDebug) {
+			$this->controller->set($this->error->getAttributes());
+		}
+
+		$this->_outputMessage($template);
 	}
 
 /**
- * Convenience method to display a 400 series page.
+ * Get method name
  *
- * @param Exception $error
- * @return void
+ * @param \Exception $exception
+ * @return string
+ */
+	protected function _method(\Exception $exception) {
+		list(, $baseClass) = namespaceSplit(get_class($exception));
+		$baseClass = substr($baseClass, 0, -9);
+		$method = Inflector::variable($baseClass) ?: 'error500';
+		return $this->method = $method;
+	}
+
+/**
+ * Get error message.
+ *
+ * @param \Exception $exception Exception
+ * @param int $code Error code
+ * @return string Error message
  */
-	public function error400($error) {
-		$message = $error->getMessage();
-		if (!Configure::read('debug') && $error instanceof Exception) {
-			$message = __d('cake', 'Not Found');
+	protected function _message(\Exception $exception, $code) {
+		$message = $this->error->getMessage();
+
+		if (!Configure::read('debug') &&
+			!($exception instanceof Error\HttpException)
+		) {
+			if ($code < 500) {
+				$message = __d('cake', 'Not Found');
+			} else {
+				$message = __d('cake', 'An Internal Error Has Occurred.');
+			}
 		}
-		$url = $this->controller->request->here();
-		$this->controller->response->statusCode($error->getCode());
-		$this->controller->set(array(
-			'message' => h($message),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('message', 'url')
-		));
-		$this->_outputMessage('error400');
+
+		return $message;
 	}
 
 /**
- * Convenience method to display a 500 page.
+ * Get template for rendering exception info.
  *
- * @param \Exception $error
- * @return void
+ * @param \Exception $exception
+ * @param string $method Method name
+ * @param int $code Error code
+ * @return string Template name
  */
-	public function error500($error) {
-		$message = $error->getMessage();
-		if (!Configure::read('debug')) {
-			$message = __d('cake', 'An Internal Error Has Occurred.');
+	protected function _template(\Exception $exception, $method, $code) {
+		$isHttpException = $exception instanceof Error\HttpException;
+
+		if (!Configure::read('debug') && !$isHttpException) {
+			$template = 'error500';
+			if ($code < 500) {
+				$template = 'error400';
+			}
+			return $this->template = $template;
 		}
-		$url = $this->controller->request->here();
-		$code = ($error->getCode() > 500 && $error->getCode() < 506) ? $error->getCode() : 500;
-		$this->controller->response->statusCode($code);
-		$this->controller->set(array(
-			'message' => h($message),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('message', 'url')
-		));
-		$this->_outputMessage('error500');
+
+		if ($isHttpException) {
+			$template = 'error500';
+			if ($code < 500) {
+				$template = 'error400';
+			}
+			return $this->template = $template;
+		}
+
+		$template = $method ?: 'error500';
+
+		if ($exception instanceof \PDOException) {
+			$template = 'pdo_error';
+		}
+
+		return $this->template = $template;
 	}
 
 /**
- * Convenience method to display a PDOException.
+ * Get an error code value within range 400 to 506
  *
- * @param \PDOException $error
- * @return void
+ * @param \Exception $exception Exception
+ * @return int Error code value within range 400 to 506
  */
-	public function pdoError(\PDOException $error) {
-		$url = $this->controller->request->here();
+	protected function _code(\Exception $exception) {
 		$code = 500;
-		$this->controller->response->statusCode($code);
-		$this->controller->set(array(
-			'code' => $code,
-			'message' => h($error->getMessage()),
-			'url' => h($url),
-			'error' => $error,
-			'_serialize' => array('code', 'message', 'url', 'error')
-		));
-		$this->_outputMessage($this->template);
+		$errorCode = $exception->getCode();
+		if ($errorCode >= 400 && $errorCode < 506) {
+			$code = $errorCode;
+		}
+		return $code;
 	}
 
 /**

src/Error/HttpException.php

@@ -19,5 +19,5 @@ namespace Cake\Error;
  * catch blocks can be specifically typed.
  *
  */
-class HttpException extends BaseException {
+class HttpException extends Exception {
 }

tests/TestCase/Error/ExceptionRendererTest.php

@@ -132,6 +132,9 @@ class MissingWidgetThingException extends Error\NotFoundException {
  */
 class ExceptionRendererTest extends TestCase {
 
+/**
+ * @var bool
+ */
 	protected $_restoreError = false;
 
 /**
@@ -201,12 +204,11 @@ class ExceptionRendererTest extends TestCase {
 		$exception = new MissingWidgetThingException('Widget not found');
 		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
 
-		$this->assertEquals('missingWidgetThing', $ExceptionRenderer->method);
-
 		ob_start();
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
+		$this->assertEquals('missingWidgetThing', $ExceptionRenderer->method);
 		$this->assertEquals('widget thing is missing', $result, 'Method declared in subclass converted to error400');
 	}
 
@@ -221,8 +223,6 @@ class ExceptionRendererTest extends TestCase {
 		$exception = new MissingControllerException('PostsController');
 		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
 
-		$this->assertEquals('error400', $ExceptionRenderer->method);
-
 		ob_start();
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
@@ -240,23 +240,29 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer = new ExceptionRenderer($exception);
 
 		$this->assertInstanceOf('Cake\Controller\ErrorController', $ExceptionRenderer->controller);
-		$this->assertEquals('error400', $ExceptionRenderer->method);
 		$this->assertEquals($exception, $ExceptionRenderer->error);
 	}
 
 /**
- * test that method gets coerced when debug = 0
+ * test that exception message gets coerced when debug = 0
  *
  * @return void
  */
-	public function testErrorMethodCoercion() {
+	public function testExceptionMessageCoercion() {
 		Configure::write('debug', false);
-		$exception = new MissingActionException('Page not found');
+		$exception = new MissingActionException('Secret info not to be leaked');
 		$ExceptionRenderer = new ExceptionRenderer($exception);
 
 		$this->assertInstanceOf('Cake\Controller\ErrorController', $ExceptionRenderer->controller);
-		$this->assertEquals('error400', $ExceptionRenderer->method);
 		$this->assertEquals($exception, $ExceptionRenderer->error);
+
+		ob_start();
+		$ExceptionRenderer->render();
+		$result = ob_get_clean();
+
+		$this->assertEquals('error400', $ExceptionRenderer->template);
+		$this->assertContains('Not Found', $result);
+		$this->assertNotContains('Secret info not to be leaked', $result);
 	}
 
 /**
@@ -291,7 +297,6 @@ class ExceptionRendererTest extends TestCase {
 		$result = ob_get_clean();
 
 		$this->assertFalse(method_exists($ExceptionRenderer, 'missingWidgetThing'), 'no method should exist.');
-		$this->assertEquals('error400', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('coding fail', $result, 'Text should show up.');
 	}
 
@@ -312,7 +317,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('foul ball.', $result, 'Text should show up as its debug mode.');
 	}
 
@@ -335,7 +339,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertNotContains('foul ball.', $result, 'Text should no show up.');
 		$this->assertContains('Internal Error', $result, 'Generic message only.');
 	}
@@ -355,7 +358,6 @@ class ExceptionRendererTest extends TestCase {
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
-		$this->assertEquals('error500', $ExceptionRenderer->method, 'incorrect method coercion.');
 		$this->assertContains('foul ball.', $result, 'Text should show up as its debug mode.');
 	}
 
@@ -480,12 +482,13 @@ class ExceptionRendererTest extends TestCase {
 			'prefix' => '',
 			'plugin' => '',
 		));
-		$ExceptionRenderer = $this->_mockResponse(new ExceptionRenderer($exception));
+		$ExceptionRenderer = $this->_mockResponse(new MyCustomExceptionRenderer($exception));
 
 		ob_start();
 		$ExceptionRenderer->render();
 		$result = ob_get_clean();
 
+		$this->assertEquals('missingController', $ExceptionRenderer->template);
 		$this->assertRegExp('/<h2>Missing Controller<\/h2>/', $result);
 		$this->assertRegExp('/<em>PostsController<\/em>/', $result);
 	}