Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Added support for PHP7 \Throwable based exceptions #138

Merged
merged 3 commits into from
May 31, 2016
Merged

Added support for PHP7 \Throwable based exceptions #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cdf576c
Added support for PHP7 \Throwable based exceptions
maurice2k May 3, 2016
1077480
Added unittest for PHP7 throwable support
May 3, 2016
0cd4135
Added PHP 7 cleanup todos
maurice2k May 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/DispatchListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ public function onDispatch(MvcEvent $e)
} catch (InvalidServiceException $exception) {
$return = $this->marshalControllerNotFoundEvent($application::ERROR_CONTROLLER_INVALID, $controllerName, $e, $application, $exception);
return $this->complete($return, $e);
} catch (\Exception $exception) {
} catch (\Throwable $exception) {
$return = $this->marshalBadControllerEvent($controllerName, $e, $application, $exception);
return $this->complete($return, $e);
} catch (\Exception $exception) { // @TODO clean up once PHP 7 requirement is enforced
$return = $this->marshalBadControllerEvent($controllerName, $e, $application, $exception);
return $this->complete($return, $e);
}
Expand All @@ -109,15 +112,22 @@ public function onDispatch(MvcEvent $e)

$request = $e->getRequest();
$response = $application->getResponse();
$caughtException = null;

try {
$return = $controller->dispatch($request, $response);
} catch (\Exception $ex) {
} catch (\Throwable $ex) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't Exception be caught prior to Throwable? As Exception extends Throwable and would therefore never be caught?

Copy link
Contributor Author

@maurice2k maurice2k May 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes no difference as the code is the same for \Exception and \Throwable. In PHP 7+ the exceptions or error exceptions are caught by \Throwable whereas in PHP 5 the exceptions are caught by \Exception.

\Exception should be removed at some point in the future (wenn PHP5 support is dropped).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a // @TODO clean up once PHP 7 requirement is enforced?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, just added the todos.

$caughtException = $ex;
} catch (\Exception $ex) { // @TODO clean up once PHP 7 requirement is enforced
$caughtException = $ex;
}

if ($caughtException !== null) {
$e->setName(MvcEvent::EVENT_DISPATCH_ERROR);
$e->setError($application::ERROR_EXCEPTION);
$e->setController($controllerName);
$e->setControllerClass(get_class($controller));
$e->setParam('exception', $ex);
$e->setParam('exception', $caughtException);

$return = $application->getEventManager()->triggerEvent($e)->last();
if (! $return) {
Expand All @@ -135,7 +145,7 @@ public function reportMonitorEvent(MvcEvent $e)
{
$error = $e->getError();
$exception = $e->getParam('exception');
if ($exception instanceof \Exception) {
if ($exception instanceof \Exception || $exception instanceof \Throwable) { // @TODO clean up once PHP 7 requirement is enforced
zend_monitor_custom_event_ex($error, $exception->getMessage(), 'Zend Framework Exception', ['code' => $exception->getCode(), 'trace' => $exception->getTraceAsString()]);
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/MiddlewareListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,17 @@ public function onDispatch(MvcEvent $event)
$event->setResult($return);
return $return;
}

$caughtException = null;
try {
$return = $middleware(Psr7Request::fromZend($request), Psr7Response::fromZend($response));
} catch (\Exception $exception) {
} catch (\Throwable $exception) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: TODO would be good

$caughtException = $exception;
} catch (\Exception $exception) { // @TODO clean up once PHP 7 requirement is enforced
$caughtException = $exception;
}

if ($caughtException !== null) {
$event->setName(MvcEvent::EVENT_DISPATCH_ERROR);
$event->setError($application::ERROR_EXCEPTION);
$event->setController($middlewareName);
Expand Down
2 changes: 1 addition & 1 deletion src/View/Console/ExceptionStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public function prepareExceptionViewModel(MvcEvent $e)
if (is_callable($this->message)) {
$callback = $this->message;
$message = (string) $callback($exception, $this->displayExceptions);
} elseif ($this->displayExceptions && $exception instanceof \Exception) {
} elseif ($this->displayExceptions && ($exception instanceof \Exception || $exception instanceof \Throwable)) { // @TODO clean up once PHP 7 requirement is enforced
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a related issue on zend-mvc-console, please? This functionality is moved into there for the v3 series.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind; I'm working on it now.

$previous = '';
$previousException = $exception->getPrevious();
while ($previousException) {
Expand Down
2 changes: 1 addition & 1 deletion src/View/Console/RouteNotFoundStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ protected function reportNotFoundReason($e)
];
$report = sprintf("\nReason for failure: %s\n", $reasons[$reason]);

while ($exception instanceof \Exception) {
while ($exception instanceof \Exception || $exception instanceof \Throwable) { // @TODO clean up once PHP 7 requirement is enforced
$report .= sprintf("Exception: %s\nTrace:\n%s\n", $exception->getMessage(), $exception->getTraceAsString());
$exception = $exception->getPrevious();
}
Expand Down
10 changes: 9 additions & 1 deletion src/View/Http/DefaultRenderingStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,17 @@ public function render(MvcEvent $e)
$view->setRequest($request);
$view->setResponse($response);

$caughtException = null;

try {
$view->render($viewModel);
} catch (\Exception $ex) {
} catch (\Throwable $ex) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

$caughtException = $ex;
} catch (\Exception $ex) { // @TODO clean up once PHP 7 requirement is enforced
$caughtException = $ex;
}

if ($caughtException !== null) {
if ($e->getName() === MvcEvent::EVENT_RENDER_ERROR) {
throw $ex;
}
Expand Down
2 changes: 1 addition & 1 deletion src/View/Http/RouteNotFoundStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ protected function injectException($model, $e)
$model->setVariable('display_exceptions', true);

$exception = $e->getParam('exception', false);
if (!$exception instanceof \Exception) {
if (!$exception instanceof \Exception && !$exception instanceof \Throwable) { // @TODO clean up once PHP 7 requirement is enforced
return;
}

Expand Down
23 changes: 21 additions & 2 deletions test/ApplicationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ public function setupActionController()
return $this->application;
}

public function setupBadController($addService = true)
public function setupBadController($addService = true, $action = 'test')
{
$request = $this->serviceManager->get('Request');
$request->setUri('http://example.local/bad');
Expand All @@ -301,7 +301,7 @@ public function setupBadController($addService = true)
'route' => '/bad',
'defaults' => [
'controller' => 'bad',
'action' => 'test',
'action' => $action,
],
]);
$router->addRoute('bad', $route);
Expand Down Expand Up @@ -374,6 +374,25 @@ public function testLocatorExceptionShouldTriggerDispatchError()
$this->assertSame($response, $result->getResponse(), get_class($result));
}

/**
* @requires PHP 7.0
* @group error-handling
*/
public function testPhp7ErrorRaisedInDispatchableShouldRaiseDispatchErrorEvent()
{
$this->setupBadController(true, 'test-php7-error');
$response = $this->application->getResponse();
$events = $this->application->getEventManager();
$events->attach(MvcEvent::EVENT_DISPATCH_ERROR, function ($e) use ($response) {
$exception = $e->getParam('exception');
$response->setContent($exception->getMessage());
return $response;
});

$this->application->run();
$this->assertContains('Raised an error', $response->getContent());
}

/**
* @group error-handling
*/
Expand Down
5 changes: 5 additions & 0 deletions test/Controller/TestAsset/BadController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,9 @@ public function testAction()
{
throw new \Exception('Raised an exception');
}

public function testPhp7ErrorAction()
{
throw new \Error('Raised an error');
}
}