[NFR] Allow action forwarding anywhere inside dispatch loop following Exception handling in "dispatch:beforeException"

[temuri416]

See #2851 for more info.

From the official documentation at https://docs.phalconphp.com/en/latest/reference/dispatching.html#handling-not-found-exceptions:

           "dispatch:beforeException",
            function (Event $event, $dispatcher, Exception $exception) {
                if ($exception instanceof DispatchException) {
                    $dispatcher->forward(
                        [
                            "controller" => "index",
                            "action"     => "show404",
                        ]
                    );
                    return false;
                }

The above code does not work as it is only called from outside the dispatch loop.

It would be very valuable to be able to forward controller actions from inside dispatch:beforeException event handlers.

At the moment the only case when the forwarding works is if Exception was thrown inside controllers.

Thanks.

[Jurigag]

I agree for dispatch loop, but in before dispatch loop and after forward should not be possible.

[temuri416]

@Jurigag Absolutely.

Only between https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L390 and https://github.com/phalcon/cphalcon/blob/master/phalcon/dispatcher.zep#L625.

I expect the following hooks to work:

• dispatch:beforeDispatch
• dispatch:beforeNotFoundAction
• dispatch:beforeExecuteRoute
• dispatch:afterInitialize
• dispatch:afterExecuteRoute
• dispatch:afterDispatch

Is there a reason why this couldn't be achieved by wrapping in try/catch as follows in example below for dispatch:beforeExecuteRoute:

try {
    if eventsManager->fire("dispatch:beforeExecuteRoute", this) === false {
        continue;
    }
} catch \Exception, e {
    if this->{"_handleException"}(e) === false {
        if this->_finished === false {
            continue;
        }
    } else {
        throw e;
    }
}

It looks like a straightforward fix to me, unless I'm missing something.

Thanks!

[virgofx]

@temuri416 THis is already implemented in upcoming bugfix for dispatcher the provides proper event throwing anywhere inside the dispatch sequence. Forthcoming documentation will update the current dispatcher.rst page with improved clarification.

mark as closed . Reference #11819 and Pull request - 12209

[temuri416]

Duplicate of #11819.

[virgofx]

@temuri416 You can see the screenshot here of all the tests covered in the PR which include exceptions and events in the beforeDispatchLoop (and afterDispatchLoop). Your case above is covered as well as a huge amount of cases that may or may not have worked at one point but got mangled overtime due to refactoring. If there any any missing please let me know.

#12209

[temuri416]

Thanks a lot. IMO this is a fix of the huge importance. Looking forward to resolution.

Gonna take a look at the PR for details...

Cheers.

[virgofx]

Fixing core things like this has exposed some flaws in Zephir ... specifically double exceptions and memory mapping of variables that is causing a segfault. So as soon as we can get this fixed upstream, we can get tests to pass, update docs, and this will be one of last things in 3.0.2

[Jurigag]

@virgofx still keep in mind, that from logic point of view forwarding from beforeDispatchLoop or afterDispatchLoop should not be possible. I hope it's not in your PR.

[virgofx]

It is, and it does make sense.

[Jurigag]

How it make sense ? Why we can forward action if we not in dispatch loop ?