[Bug] \Phalcon\Mvc\Dispatcher does not set Last Handler on Exception

[virgofx]

When an exception occurs dispatching, the last handler will not get set. The setting of the last handler is only performed on success but needs to happen in all cases as subsequent error handling actions further up the stack that may dispatch again would require this information (as in the case in one of my error handling applications) This could easily be achieved with a finally block for the try/catch and moving the setting of the last handler into that section.

See
https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L510

[rianorie]

Can you produce a script that reproduces the behavior?

[virgofx]

IndexController.php

class IndexController extends \Phalcon\Mvc\Controller
{
    public function indexAction() {
        throw new \MyCustomException('custom exception');
    }
}

bootstrap.php

$di = new \Phalcon\Di\FactoryDefault();
try {
        $application = new \Phalcon\Application();
        $application->setEventsManager(new \Phalcon\Events\Manager()));
        $application->setDI($di);

        echo $application->handle()->getContent();

    } catch (\MyCustomException $e) {
        // "IndexController"  [OK]
        var_dump($di->getDispatcher()->getActiveController());

        // [empty]  [WRONG]
        // Expected: "IndexController"
        var_dump($di->getDispatcher()->getLastController());

        // Custom logic here to send errors or do something custom that requires
        // knowledge of the last controller.
    }
}

The Fix
https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L510

Change

try {
    // We update the latest value produced by the latest handler
    let this->_returnedValue = call_user_func_array([handler, actionMethod], params),
        this->_lastHandler = handler;

} catch \Exception, e {
    if this->{"_handleException"}(e) === false {
        if this->_finished === false {
            continue;
        }
    } else {
        throw e;
    }
}

To

try {
    // We update the latest value produced by the latest handler
    let this->_returnedValue = call_user_func_array([handler, actionMethod], params);

} catch \Exception, e {
    if this->{"_handleException"}(e) === false {
        if this->_finished === false {
            continue;
        }
    } else {
        throw e;
    }
} finally {
    this->_lastHandler = handler;
}

In the first block, the last handler does not get set if an exception occurs. I've moved this logic into a finally() block; the same thing could be achieved by duplicating the line in the catch block (although, slightly less clean).

[rianorie]

.. you've adjusted the zephir code as such? Then why not make it a pull request? :)

I'm happy to do it, though

[virgofx]

I've extended in PHP land for my projects, which do essentially above ^^. I currently don't have Zephir checked out , which is why I haven't done it yet. If you could add the fix, that'd be great ;)

[sergeyklay]

Fixed in 2.0.x