Skip to content
Permalink
Browse files

Refactor exception handling to return instead of echoing.

By returning a response/string instead of directly echoing we get a few
benefits:

1. We can more easily test the code without relying on ob_start().
2. Application exception handlers can have full control over the
   response headers.
3. We can fix issues related to nested error pages.

Refs #4130
  • Loading branch information...
markstory committed Aug 26, 2014
1 parent d3e760f commit 178ad3494218cd79af8b7614398ae90f98402c0f
@@ -184,15 +184,11 @@ public function handleFatalError($code, $description, $file, $line) {
];
$this->_logError(LOG_ERR, $data);
if (ob_get_level()) {
ob_end_clean();
}
if (Configure::read('debug')) {
$this->handleException(new FatalErrorException($description, 500, $file, $line));
} else {
$this->handleException(new InternalErrorException());
return true;
}
$this->handleException(new InternalErrorException());
return true;
}
@@ -135,7 +135,9 @@ protected function _displayException($exception) {
throw new \Exception("$renderer is an invalid class.");
}
$error = new $renderer($exception);
$error->render();
$response = $error->render();
$this->_clearOutput();
$this->_sendResponse($response);
} catch (\Exception $e) {
// Disable trace for internal errors.
$this->_options['trace'] = false;
@@ -148,4 +150,31 @@ protected function _displayException($exception) {
}
}
/**
* Clear output buffers so error pages display properly.
*
* Easily stubbed in testing.
*
* @return void
*/
protected function _clearOutput() {
while (ob_get_level()) {
ob_end_clean();
}
}
/**
* Method that can be easily stubbed in testing.
*
* @param string|Cake\Network\Response $response Either the message or response object.
* @return void
*/
protected function _sendResponse($response) {
if (is_string($response)) {
echo $response;
return;
}
echo $response->send();
}
}
@@ -118,7 +118,7 @@ protected function _getController() {
/**
* Renders the response for the exception.
*
* @return void
* @return Cake\Network\Response The response to be sent.
*/
public function render() {
$exception = $this->error;
@@ -130,8 +130,7 @@ public function render() {
if (($isDebug || $exception instanceof Error\HttpException) &&
method_exists($this, $method)
) {
call_user_func_array(array($this, $method), array($exception));
return;
return $this->_customMethod($method, $exception);
}
$message = $this->_message($exception, $code);
@@ -152,9 +151,24 @@ public function render() {
if ($exception instanceof Error\Exception && $isDebug) {
$this->controller->set($this->error->getAttributes());
}
$this->_outputMessage($template);
return $this->_outputMessage($template);
}
/**
* Render a custom error method/template.
*
* @param string $method The method name to invoke.
* @param \Exception $exception The exception to render.
* @return \Cake\Network\Response The response to send.
*/
protected function _customMethod($method, $exception) {
$result = call_user_func([$this, $method], $exception);
if (is_string($result)) {
$this->controller->response->body($result);
$result = $this->controller->response;
}
return $result;
}
/**
* Get method name
*
@@ -246,29 +260,28 @@ protected function _code(\Exception $exception) {
* Generate the response using the controller object.
*
* @param string $template The template to render.
* @return void
* @return Cake\Network\Response A response object that can be sent.
*/
protected function _outputMessage($template) {
try {
$this->controller->render($template);
$event = new Event('Controller.shutdown', $this->controller);
$this->controller->afterFilter($event);
$this->controller->response->send();
return $this->controller->response;
} catch (MissingViewException $e) {
$attributes = $e->getAttributes();
if (isset($attributes['file']) && strpos($attributes['file'], 'error500') !== false) {
$this->_outputMessageSafe('error500');
} else {
$this->_outputMessage('error500');
return $this->_outputMessageSafe('error500');
}
return $this->_outputMessage('error500');
} catch (MissingPluginException $e) {
$attributes = $e->getAttributes();
if (isset($attributes['plugin']) && $attributes['plugin'] === $this->controller->plugin) {
$this->controller->plugin = null;
}
$this->_outputMessageSafe('error500');
return $this->_outputMessageSafe('error500');
} catch (\Exception $e) {
$this->_outputMessageSafe('error500');
return $this->_outputMessageSafe('error500');
}
}
@@ -289,7 +302,7 @@ protected function _outputMessageSafe($template) {
$view = $this->controller->createView();
$this->controller->response->body($view->render($template, 'error'));
$this->controller->response->type('html');
$this->controller->response->send();
return $this->controller->response;
}
}
@@ -1,7 +1,5 @@
<?php
/**
* ErrorHandlerTest file
*
* CakePHP(tm) Tests <http://book.cakephp.org/2.0/en/development/testing.html>
* Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
*
@@ -30,24 +28,34 @@
use Cake\TestSuite\TestCase;
/**
* Test exception renderer
* Testing stub.
*/
class TestExceptionRenderer extends ExceptionRenderer {
class TestErrorHandler extends ErrorHandler {
/**
* Constructor for mocking Response::_sendHeader()
* Access the response used.
*
* @param \Exception $exception
* @var \Cake\Network\Response
*/
public function __construct(\Exception $exception) {
parent::__construct($exception);
$testCase = new ErrorHandlerTest();
$this->controller->response = $testCase->getMock(
'Cake\Network\Response',
['_sendHeader']
);
public $response;
/**
* Stub output clearing in tests.
*
* @return void
*/
protected function _clearOutput() {
// noop
}
/**
* Stub sending responses
*
* @return void
*/
protected function _sendResponse($response) {
$this->response = $response;
}
}
/**
@@ -137,9 +145,9 @@ public function testErrorMapping($error, $expected) {
ob_start();
trigger_error('Test error', $error);
$result = ob_get_clean();
$this->assertRegExp('/<b>' . $expected . '<\/b>/', $result);
$this->assertContains('<b>' . $expected . '</b>', $result);
}
/**
@@ -213,14 +221,10 @@ public function testHandleErrorLoggingTrace() {
*/
public function testHandleException() {
$error = new Error\NotFoundException('Kaboom!');
$errorHandler = new ErrorHandler([
'exceptionRenderer' => 'Cake\Test\TestCase\Error\TestExceptionRenderer'
]);
$errorHandler = new TestErrorHandler();
ob_start();
$errorHandler->handleException($error);
$result = ob_get_clean();
$this->assertRegExp('/Kaboom!/', $result, 'message missing.');
$this->assertContains('Kaboom!', $errorHandler->response->body(), 'message missing.');
}
/**
@@ -229,9 +233,8 @@ public function testHandleException() {
* @return void
*/
public function testHandleExceptionLog() {
$errorHandler = new ErrorHandler([
$errorHandler = new TestErrorHandler([
'log' => true,
'exceptionRenderer' => 'Cake\Test\TestCase\Error\TestExceptionRenderer'
]);
$error = new Error\NotFoundException('Kaboom!');
@@ -243,10 +246,8 @@ public function testHandleExceptionLog() {
$this->stringContains('ErrorHandlerTest->testHandleExceptionLog')
));
ob_start();
$errorHandler->handleException($error);
$result = ob_get_clean();
$this->assertRegExp('/Kaboom!/', $result, 'message missing.');
$this->assertContains('Kaboom!', $errorHandler->response->body(), 'message missing.');
}
/**
@@ -265,21 +266,16 @@ public function testHandleExceptionLogSkipping() {
$this->stringContains('[Cake\Error\ForbiddenException] Fooled you!')
);
$errorHandler = new ErrorHandler([
$errorHandler = new TestErrorHandler([
'log' => true,
'skipLog' => ['Cake\Error\NotFoundException'],
'exceptionRenderer' => 'Cake\Test\TestCase\Error\TestExceptionRenderer'
]);
ob_start();
$errorHandler->handleException($notFound);
$result = ob_get_clean();
$this->assertRegExp('/Kaboom!/', $result, 'message missing.');
$this->assertContains('Kaboom!', $errorHandler->response->body(), 'message missing.');
ob_start();
$errorHandler->handleException($forbidden);
$result = ob_get_clean();
$this->assertRegExp('/Fooled you!/', $result, 'message missing.');
$this->assertContains('Fooled you!', $errorHandler->response->body(), 'message missing.');
}
/**
@@ -289,16 +285,15 @@ public function testHandleExceptionLogSkipping() {
*/
public function testLoadPluginHandler() {
Plugin::load('TestPlugin');
$errorHandler = new ErrorHandler([
$errorHandler = new TestErrorHandler([
'exceptionRenderer' => 'TestPlugin.TestPluginExceptionRenderer',
]);
$error = new Error\NotFoundException('Kaboom!');
ob_start();
$errorHandler->handleException($error);
$result = ob_get_clean();
$result = $errorHandler->response;
$this->assertEquals('Rendered by test plugin', $result);
Plugin::unload();
}
/**
@@ -310,23 +305,18 @@ public function testLoadPluginHandler() {
*/
public function testHandleFatalErrorPage() {
$line = __LINE__;
$errorHandler = new ErrorHandler([
'exceptionRenderer' => 'Cake\Test\TestCase\Error\TestExceptionRenderer'
]);
$errorHandler = new TestErrorHandler();
Configure::write('debug', true);
ob_start();
ob_start();
$errorHandler->handleFatalError(E_ERROR, 'Something wrong', __FILE__, $line);
$result = ob_get_clean();
$result = $errorHandler->response->body();
$this->assertContains('Something wrong', $result, 'message missing.');
$this->assertContains(__FILE__, $result, 'filename missing.');
$this->assertContains((string)$line, $result, 'line missing.');
ob_start();
ob_start();
Configure::write('debug', false);
$errorHandler->handleFatalError(E_ERROR, 'Something wrong', __FILE__, $line);
$result = ob_get_clean();
$result = $errorHandler->response->body();
$this->assertNotContains('Something wrong', $result, 'message must not appear.');
$this->assertNotContains(__FILE__, $result, 'filename must not appear.');
$this->assertContains('An Internal Error Has Occurred', $result);
@@ -341,21 +331,16 @@ public function testHandleFatalErrorLog() {
$this->_logger->expects($this->at(0))
->method('write')
->with('error', $this->logicalAnd(
$this->stringContains(__FILE__ . ', line ' . (__LINE__ + 13)),
$this->stringContains(__FILE__ . ', line ' . (__LINE__ + 9)),
$this->stringContains('Fatal Error (1)'),
$this->stringContains('Something wrong')
));
$this->_logger->expects($this->at(1))
->method('write')
->with('error', $this->stringContains('[Cake\Error\FatalErrorException] Something wrong'));
$errorHandler = new ErrorHandler([
'log' => true,
'exceptionRenderer' => 'Cake\Test\TestCase\Error\TestExceptionRenderer'
]);
ob_start();
$errorHandler = new TestErrorHandler(['log' => true]);
$errorHandler->handleFatalError(E_ERROR, 'Something wrong', __FILE__, __LINE__);
ob_clean();
}
}
Oops, something went wrong.

0 comments on commit 178ad34

Please sign in to comment.
You can’t perform that action at this time.