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

Catch exceptions thrown during rendering #2812

Closed
wants to merge 11 commits into from
Closed

Catch exceptions thrown during rendering #2812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fd03686
Catch render exceptions and trigger dispatch error
radnan Oct 19, 2012
743a06c
Catch exception, cleanup, and re-throw when rendering model
Oct 19, 2012
257941f
Clear child models before injecting in case of error
Oct 19, 2012
4f1d1d3
clear out child models
Oct 19, 2012
682137e
Implemented clearChildren()
Oct 19, 2012
c89b5e1
Opening brace on same-line
radnan Oct 20, 2012
b98ca20
Trigger render_error event
radnan Oct 20, 2012
5091be1
Added render.error event constant
radnan Oct 20, 2012
2b5b708
Attach to render.error
radnan Oct 20, 2012
50a5f39
Attach to 'render.error'
radnan Oct 20, 2012
15e8718
Removed unnecessary cleanup
radnan Oct 20, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion library/Zend/Mvc/Application.php
View file
Open in desktop
Expand Up @@ -323,7 +323,14 @@ protected function completeRequest(MvcEvent $event)
{
$events = $this->getEventManager();
$event->setTarget($this);
$events->trigger(MvcEvent::EVENT_RENDER, $event);
try {
$events->trigger(MvcEvent::EVENT_RENDER, $event);
} catch (\Exception $ex) {
$event->setError($this::ERROR_EXCEPTION)
->setParam('exception', $ex);
$events->trigger(MvcEvent::EVENT_RENDER_ERROR, $event);
$events->trigger(MvcEvent::EVENT_RENDER, $event);
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we should do the same as we do with route and dispatch, and create a Zend\Mvc\RenderListener that encapsulates this. It'll be more consistent, allow swapping out internals more easily, and keep the functionality more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 with the RenderListener to keep consistency & readable.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, on investigating this, we already have a "RenderListener" -- it's the DefaultRenderingStrategy. What we need to do is treat this like we do dispatch and route: push the exception handling into these default listeners (I use the plural, as we have both Console and HTTP variants). I'm tackling this now.

$events->trigger(MvcEvent::EVENT_FINISH, $event);
return $event->getResponse();
}
Expand Down
1 change: 1 addition & 0 deletions library/Zend/Mvc/MvcEvent.php
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@ class MvcEvent extends Event
const EVENT_DISPATCH_ERROR = 'dispatch.error';
const EVENT_FINISH = 'finish';
const EVENT_RENDER = 'render';
const EVENT_RENDER_ERROR = 'render.error';
const EVENT_ROUTE = 'route';
/**#@-*/

Expand Down
1 change: 1 addition & 0 deletions library/Zend/Mvc/View/Console/ExceptionStrategy.php
View file
Open in desktop
Expand Up @@ -61,6 +61,7 @@ class ExceptionStrategy implements ListenerAggregateInterface
public function attach(EventManagerInterface $events)
{
$this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH_ERROR, array($this, 'prepareExceptionViewModel'));
$this->listeners[] = $events->attach(MvcEvent::EVENT_RENDER_ERROR, array($this, 'prepareExceptionViewModel'));
}

/**
Expand Down
1 change: 1 addition & 0 deletions library/Zend/Mvc/View/Console/ViewManager.php
View file
Open in desktop
Expand Up @@ -67,6 +67,7 @@ public function onBootstrap($event)
$events->attach($routeNotFoundStrategy);
$events->attach($exceptionStrategy);
$events->attach(MvcEvent::EVENT_DISPATCH_ERROR, array($injectViewModelListener, 'injectViewModel'), -100);
$events->attach(MvcEvent::EVENT_RENDER_ERROR, array($injectViewModelListener, 'injectViewModel'), -100);
$events->attach($mvcRenderingStrategy);
$events->attach($sendResponseListener);

Expand Down
1 change: 1 addition & 0 deletions library/Zend/Mvc/View/Http/ExceptionStrategy.php
View file
Open in desktop
Expand Up @@ -51,6 +51,7 @@ class ExceptionStrategy implements ListenerAggregateInterface
public function attach(EventManagerInterface $events)
{
$this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH_ERROR, array($this, 'prepareExceptionViewModel'));
$this->listeners[] = $events->attach(MvcEvent::EVENT_RENDER_ERROR, array($this, 'prepareExceptionViewModel'));
}

/**
Expand Down
4 changes: 4 additions & 0 deletions library/Zend/Mvc/View/Http/InjectViewModelListener.php
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@ public function attach(Events $events)
{
$this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH, array($this, 'injectViewModel'), -100);
$this->listeners[] = $events->attach(MvcEvent::EVENT_DISPATCH_ERROR, array($this, 'injectViewModel'), -100);
$this->listeners[] = $events->attach(MvcEvent::EVENT_RENDER_ERROR, array($this, 'injectViewModel'), -100);
}

/**
Expand Down Expand Up @@ -83,6 +84,9 @@ public function injectViewModel(MvcEvent $e)
return;
}

if ($e->getError()) {
$model->clearChildren();
}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should do this, or provide an alternate layout view model instead?

$model->addChild($result);
}
}
1 change: 1 addition & 0 deletions library/Zend/Mvc/View/Http/ViewManager.php
View file
Open in desktop
Expand Up @@ -146,6 +146,7 @@ public function onBootstrap($event)
$events->attach($routeNotFoundStrategy);
$events->attach($exceptionStrategy);
$events->attach(MvcEvent::EVENT_DISPATCH_ERROR, array($injectViewModelListener, 'injectViewModel'), -100);
$events->attach(MvcEvent::EVENT_RENDER_ERROR, array($injectViewModelListener, 'injectViewModel'), -100);
$events->attach($mvcRenderingStrategy);
$events->attach($sendResponseListener);

Expand Down
7 changes: 7 additions & 0 deletions library/Zend/View/Model/ModelInterface.php
View file
Open in desktop
Expand Up @@ -127,6 +127,13 @@ public function getChildren();
*/
public function hasChildren();

/**
* Clears out all child models
*
* @return ModelInterface
*/
public function clearChildren();
Copy link
Member

Choose a reason for hiding this comment

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

This is a backwards incompatible break. We should add it as a separate interface, and then implement it in the concrete implementations we have; anything that requires its existence would check against it before operating on it.


/**
* Set the name of the variable to capture this model to, if it is a child model
*
Expand Down
11 changes: 11 additions & 0 deletions library/Zend/View/Model/ViewModel.php
View file
Open in desktop
Expand Up @@ -351,6 +351,17 @@ public function hasChildren()
return (0 < count($this->children));
}

/**
* Clears out all child models
*
* @return ViewModel
*/
public function clearChildren()
{
$this->children = array();
return $this;
}

/**
* Set the name of the variable to capture this model to, if it is a child model
*
Expand Down
11 changes: 8 additions & 3 deletions library/Zend/View/Renderer/PhpRenderer.php
View file
Open in desktop
Expand Up @@ -457,9 +457,14 @@ public function render($nameOrModel, $values = null)
$this->__template
));
}
ob_start();
include $this->__file;
$this->__content = ob_get_clean();
try {
ob_start();
include $this->__file;
$this->__content = ob_get_clean();
} catch (\Exception $ex) {
ob_end_clean();
throw $ex;
}
}

$this->setVars(array_pop($this->__varsCache));
Expand Down