Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Zend\Mvc\Controller\Plugin\Forward::reattachProblemListeners #4130

Closed
turrsis opened this Issue Mar 27, 2013 · 5 comments

Comments

Projects
None yet
3 participants
Contributor

turrsis commented Mar 27, 2013

When controller->dispatch throw Exception Forward::reattachProblemListeners will never executed.
And SharedEventManager will never restore self state

Owner

weierophinney commented Mar 27, 2013

Please provide some code that recreates the issue; I'm having trouble understanding when/how the issue occurs, and what exactly it is, based on the description provided.

Contributor

turrsis commented Mar 28, 2013

First forward detachProblemListeners and reattachProblemListeners
Second forward detachProblemListeners raise Exception and not reattachProblemListeners
When call third forward SharedEventManager has no some events

public function indexAction() {
    $this->forward()->dispatch('controller1', array('some_params'));
    try{
        $this->forward()->dispatch('controller2', array('some_params'));
    } catch(\Exception $ex) {
        $q = '';
    }
    $this->forward()->dispatch('controller3', array('some_params'));
}

To my mind Zend\Mvc\Controller\Plugin\Forward::dispatch function shoud be :

public function dispatch($name, array $params = null) {
    ..........
    $listeners = $this->detachProblemListeners($sharedEvents);
    try {
        $return = $controller->dispatch($event->getRequest(), $event->getResponse());
    } catch (\Exception $ex) {
        $this->reattachProblemListeners($sharedEvents, $listeners);
        throw $ex;
    }
    // If we detached any listeners, reattach them now:
    $this->reattachProblemListeners($sharedEvents, $listeners);
    ..........
}
Contributor

turrsis commented Mar 28, 2013

And more better solution is :

namespace Zend\Mvc\Controller\Plugin;
.........
class Forward extends AbstractPlugin {
..........
public function dispatch($name, array $params = null) {
    ..........
    try {
        $return = $controller->dispatch($event->getRequest(), $event->getResponse());
    } catch (\Exception $ex) {
        if ($event->getName() === MvcEvent::EVENT_DISPATCH_ERROR) {
            $this->reattachProblemListeners($sharedEvents, $listeners);
            throw $ex;
        }
        $application = $event->getApplication();
        $event->setError($application::ERROR_EXCEPTION)
            ->setController($controller)
            ->setControllerClass(get_class($controller))
            ->setParam('exception', $ex);

        $events = $application->getEventManager();
        /*Here we need to detach Tgs\Mvc\View\Http\InjectViewModelListener::injectViewModel 
        from MvcEvent::EVENT_DISPATCH_ERROR */

        $results = $events->trigger(MvcEvent::EVENT_DISPATCH_ERROR, $event);

        /*Here we need to attach Tgs\Mvc\View\Http\InjectViewModelListener::injectViewModel 
        to MvcEvent::EVENT_DISPATCH_ERROR */

        $return = $results->last();
        if (! $return) {
            $return = $event->getResult();
        }
    }
    ..........
}

With this solution forward helper always will return ViewModel

Member

adamlundrigan commented Oct 21, 2015

This issue does still exist in the ZF2 codebase. What's the practical consequence of it? If I've grokked this part of the MVC stack properly it means that if the controller making the forward call swallows the exception and returns an array from the action it won't be converted to a ViewModel automatically (https://github.com/zendframework/zend-mvc/blob/master/src/Controller/Plugin/Forward.php#L85). I don't see a way for userland applications to inject their own listeners into the problem list, and that's the only listener on the list. That said, it is wrong and should be fixed if it's not considered a BC break (which it doesn't to me)

Contributor

turrsis commented Oct 22, 2015

this is very old issue, not actual

@turrsis turrsis closed this Oct 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment