[BUG] Action forwarding is not working in 'dispatch:beforeException'.

[temuri416]

Hi all,

It is related to this: http://docs.phalconphp.com/en/latest/reference/dispatching.html#handling-not-found-exceptions. Whenever an exception is thrown, action forwarding to Error controller does not work and results in a blank page.

Steps to repro:

• Check out Phalcon's "INVO" example.
• Create ErrorController.php:

    class ErrorController extends ControllerBase
    {
        public function indexAction()
        {
                error_log('Error Controller');
        }
    }

• Create view script "error/index.volt" with a message "KABOOM!"
• Edit /index.php and use the following code:

    /**
     * We register the events manager
     */
    $di->set('dispatcher', function() use ($di) {

        $eventsManager = $di->getShared('eventsManager');
        $eventsManager->attach('dispatch:beforeDispatchLoop', function(Phalcon\Events\Event $event, Phalcon\Mvc\Dispatcher $dispatcher) {
            throw new \Exception('Just because');
        });

        $eventsManager->attach('dispatch:beforeException', function ($event, $dispatcher, $exception) use (&$di) {
            error_log('dispatch:beforeException');
            $dispatcher->forward(
                array(
                    'controller' => 'error',
                    'action'     => 'index',
                    'error' => $exception
                )
            );
            return false;
        });

        $security = new Security($di);

        /**
         * We listen for events in the dispatcher using the Security plugin
         */
        $eventsManager->attach('dispatch', $security);

        $dispatcher = new Phalcon\Mvc\Dispatcher();
        $dispatcher->setEventsManager($eventsManager);

        return $dispatcher;
    });

Exception "Just because" is thrown and caught in 'dispatch:beforeException'. However, forwarding to /error/index does not work. Execution terminates immediately after "return false". I am using PHPEd debugger to trace it.

Also,

Can Phalcon guys explain the following cryptic sentence from the docs:

Only exceptions produced by the dispatcher and exceptions produced in the executed action are notified in the ‘beforeException’ events. Exceptions produced in listeners or controller events are redirected to the latest try/catch.

What does that really mean??

Thanks!

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[temuri416]

More info on the subject.

1. Exception thrown in any of the controller action results in a correct redirect to error/index
2. Exception thrown in "beforeDispatchLoop" results in a blank page after action is forwarded

Item 2 above is especially problematic, since it makes impossible throwing exceptions in controller plugins (ACL is a good example).

[dugwood]

I'm currently facing the same issue. But in the case of the ACL, you can simply launch the forward action:

if (!$logged) {
    $dispatcher->forward(array('namespace' => 'MyNamespace', 'controller' => 'error', 'action' => 'forbidden'));
    return false;
}

[dugwood]

Only exceptions produced by the dispatcher and exceptions produced in the executed action are notified in the ‘beforeException’ events.

Means that if you throw an exception in the dispatcher or in the controller's action, you will be able to handle the exception in 'beforeException'

Exceptions produced in listeners or controller events are redirected to the latest try/catch.

These exceptions won't go through 'beforeException'. You must use a try/catch in your function. Say:

public function afterDispatch(Event $event, Dispatcher $dispatcher)
    {
        try
        {
            // do something
            return true; // or false to stop execution
        }
        catch (\Exception $exception)
        {
            // handle the exception (do a forward...)
                        return false;
        }
    }

By the way, you helped me handling the issue: I'm using a $dispatcher->forward() instead of the Exception in my beforeDispatch code.

Also don't forget to return false when you need to forward, else the main action won't stop and you'll end up with strange things.

[titus-toia]

What I've found is that exceptions thrown in "beforeDispatchLoop" are indeed caught by "beforeException", but the dispatcher's internal redirect no longer works.

[phalcon]

Could you please try again with 2.0.0?

[mikacalvo]

In the Invo app, if we print the NotFoundPlugin Exception caused by SecurityPlugin's beforeDispatch function, we get :
"Dispatcher has detected a cyclic routing causing stability problems"

Does that mean that we can't access the errors/show401 page ? Apparently, you forgot to declare it in the $publicResources array, so I did it and the problem is still the same (same Exception, same message)

[temuri416]

@phalcon

I've finally upgraded my project to the latest Phalcon 3.x.

The issue is still there. I was debugging INVO again, and whenever execution lands in public function beforeException() action forwarding does not work.

So, again - my question is as follows:

Can I expect the following code to forward action to ErrorsController:

    public function beforeException(Event $event, MvcDispatcher $dispatcher, Exception $exception)
    {
        $dispatcher->forward(array(
            'controller' => 'errors',
            'action'     => 'show500'
        ));
        return false;
    }

The way it works now - dispatcher exits the execution loop and action is not forwarded to ErrorsController.

[Jurigag]

I think this issue is already opened somewhere. @virgofx was planning to fix this.

The issue is super obvious - https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L343

There is exception is catched, but actual dispatch loop where forward could work is happening here https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L390

This is why you can't forward from beforeException.

[sergeyklay]

@temuri416 Did you try something like this

https://github.com/phalcon/phalcon-devtools/blob/3.0.x/scripts/Phalcon/Mvc/Dispatcher/ErrorHandler.php

[Jurigag]

But your code @sergeyklay won't work too. Check above. Catch exception and handling it is happened outside dispatch loop.

[sergeyklay]

I checked it right now

[temuri416]

@sergeyklay

Looking at that code I don't see any difference from what I pasted above. In both cases a boolean is returned.

Looking at dispatcher.zip I get a feeling that it's this->_finished that needs to be FALSE...

[Jurigag]

beforeDispatchLoop event is fired OUTSIDE of dispatch loop, this is why forward in beforeException can't happen.

[sergeyklay]

the problem is - how do you use it
https://github.com/phalcon/phalcon-devtools/blob/3.0.x/scripts/Phalcon/Access/Manager.php#L79-L86

[virgofx]

@temuri416 This is a problem. There is a more recent issue - #11819 - So new follow up should reference this issue.

It's tentatively fixed and in a holding pattern waiting for a core bug in Zephir to be fixed before it will be merged. Hopefully very soon.

Also, any exception thrown in the beforeDispatchLoop does get thrown with the fix, which was a mistep with a lot of previous checkins on Dispatcher. Documentation is already ready to be submitted and will be updated as well.

[Jurigag]

@virgofx

You mean this issue i created - zephir-lang/zephir#1336 ? I guess it's possibly affecting code like:

try {

}
catch {
try {

}
catch {

}
}

too

[virgofx]

thats a separate (but closely related) issue, not related to the issue with the dispatcher fixes.

More info can be found here- zephir-lang/zephir#1325

[Jurigag]

Anyway this problem form this issue actually is not related to those issues - just current code is not allowing to forward from beforeDispatchLoop or afterDispatchLoop, no bug or something. Just forward can be done only from dispatch loop, obviously before or after it you can't do it.

[virgofx]

Yes and that's all fixed here :)
https://github.com/virgofx/cphalcon/blob/b3eaeda7e09d3b57df6fd3781b09afe1a91db458/tests/unit/Mvc/Dispatcher/DispatcherBeforeDispatchLoopTest.php#L98

[Jurigag]

Actually not fixed, just new feature to forward from beforeDispatchLoop and afterDispatchLoop :D

also update tests @virgofx because i latest checked tests are failing in your repo with those changes for old unit-tests folder

[virgofx]

Will check juri. Last I ran, everything was working, 100% on the dispatcher was working. The old dispatcher tests were removed because they're weren't accurate.

[Jurigag]

Oh then it's possible i run tests in wrong repo then.... my mistake if i did this

[temuri416]

Hi all,

I've looked closely at the execution workflow and quite honestly, don't see how to fix this without refactoring dispatch method.

The only thing that comes to mind is to enclose every event fire in its own try / catch block similar to https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L341.

Is this the intended approach?

Thanks!

[Jurigag]

Do you understand there is NO BUG ? Forward can happen only from DISPATCH LOOP, like from WHILE, you currently trying to forward outside of dispatch loop which obviously don't work. This is not bug, this is NFR, create seperated issue.

The other issue is about zephir and some other exception related stuff, but only about forward and why it's not working it's not related to this.

[temuri416]

Yes, I see this is not a bug.

Forward can happen only from DISPATCH LOOP.

Yes, right now that's the way it is. But there's no reason it could not happen from anywhere inside https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L390 loop.

[Jurigag]

But doing forward from event works fine. The problem which there is currently is throwing exception from event - this doesn't work currently with php 5.6, on php 7 there are some other bugs which can occur. And we are talking mostly about this with @virgofx Forward should work totally fine, i'm even using it extensively and there are no any problem.