[BUG] v1.3.0 - 'dispatch:beforeException' of Phalcon\Mvc\Dispatcher never fires when an Exception is thrown.

[temuri416]

Latest Phalcon v.1.3.0.

Steps to reproduce:

1. Clone https://github.com/phalcon/mvc/tree/master/simple
2. Edit index.php, use the following:

$di->set('dispatcher', function() {
    $eventsManager = new Phalcon\Events\Manager;

    $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) {
        // *** We never arrive here ***

        // Blah blah blah, do Exception logging.
    });

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

    return $dispatcher;
});

An Exception is thrown in dispatch:beforeDispatchLoop, I expect that I'd be able to capture it in dispatch:beforeException phase.

This had worked before, I've been doing Exception logging for some time.

Thanks!

[ghost]

If we call beforeException and abort the request, will this be acceptable?

[temuri416]

1. Do I understand correctly that it WAS firing before and then something changed?
2. This is my code of public/index.php:

$response = $app->handle();

foreach ($app->getConfig()['params']['log_devices'] as $device => $config) {
    if ($app->getDI()->getService($device)->isResolved()) {
        /**
         * @var Phalcon\Logger\Adapter
         */
        $device = $app->getDI()->get($device);
        if ($device->isTransaction()) {
            $device->commit();
        }
    }
}

if (isset($response)) {
    echo $response->getContent();
}

are you offering to fire dispatch:beforeException and terminate the request, so that the foreach above is never reached?

[ghost]

Do I understand correctly that it WAS firing before and then something changed?

Don't think so — I did not look back far away into the past but it looks like beforeException was called only in two cases:

1. Internal error (ie, dependency injector was not supplied or the handler class cannot be instantiated);
2. Action handler threw an exception

are you offering to fire dispatch:beforeException and terminate the request, so that the foreach above is never reached?

Not exactly. If an event handler throws an exception, we will call beforeException and exit the dispatch loop, just like as if the handler returned false.

[temuri416]

Yeah, that's fine. As long as I can capture thrown Exception and log it and then reach the end of index.php - I'm happy :-)

Thanks!

[ghost]

OK, I modified index,php as follows:

diff --git a/simple/public/index.php b/simple/public/index.php
index b1f97e0..68365c4 100644
--- a/simple/public/index.php
+++ b/simple/public/index.php
@@ -16,7 +16,22 @@ $di = new \Phalcon\DI();
 $di->set('router', 'Phalcon\Mvc\Router');

 //Registering a dispatcher
-$di->set('dispatcher', 'Phalcon\Mvc\Dispatcher');
+$di->set('dispatcher', function() {
+       $eventsManager = new Phalcon\Events\Manager;
+
+       $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) {
+               error_log('dispatch:beforeException' . PHP_EOL);
+       });
+
+       $dispatcher = new Phalcon\Mvc\Dispatcher;
+       $dispatcher->setEventsManager($eventsManager);
+
+       return $dispatcher;
+});

 //Registering a Http\Response
 $di->set('response', 'Phalcon\Http\Response');
@@ -50,6 +65,7 @@ try {
        $application = new \Phalcon\Mvc\Application();
        $application->setDI($di);
        echo $application->handle()->getContent();
+       echo 'After $application->handle()->getContent()', PHP_EOL;
 }
 catch(Phalcon\Exception $e){
        echo $e->getMessage();

The updated Phalcon is here if you want to test it: https://github.com/sjinks/cphalcon/tree/issue-1763 (I will submit a pull request later because I need to write a test first and it is late night / early morning here 😄)

The output of the script:

dispatch:beforeException

After $application->handle()->getContent()

which looks good to me

[temuri416]

Thanks! It's getting late here too (GMT-5), I will check it out tomorrow morning.

[temuri416]

I've just compiled latest 1.3.0 and this is not working for me:

$di = new \Phalcon\DI\FactoryDefault();
$di->set('dispatcher', function() {
        $eventsManager = new Phalcon\Events\Manager;

        $eventsManager->attach('dispatch:beforeDispatchLoop', function($event, $dispatcher) {
                throw new Exception('Just because.');
        });

        $eventsManager->attach('dispatch:beforeException', function($event, $dispatcher, $exception) {

                while (ob_get_level()) {
                        ob_end_clean();
                }

                echo 'dispatch:beforeException', PHP_EOL;
                ob_start();
        });

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

        return $dispatcher;
});

$di->set('view', 'Phalcon\\Mvc\\View');
$application = new \Phalcon\Mvc\Application($di);
echo $application->handle()->getContent();
echo 'After $application->handle()->getContent()', PHP_EOL;

Result:

<b>Fatal error</b>:  Uncaught exception 'Exception' with message 'Just because.' in /var/web/nginx/html/public/e.php:8
Stack trace:
#0 [internal function]: {closure}(Object(Phalcon\Events\Event), Object(Phalcon\Mvc\Dispatcher), NULL)
#1 [internal function]: Phalcon\Events\Manager-&gt;fireQueue(Array, Object(Phalcon\Events\Event))
#2 [internal function]: Phalcon\Events\Manager-&gt;fire('dispatch:before...', Object(Phalcon\Mvc\Dispatcher))
#3 [internal function]: Phalcon\Dispatcher-&gt;dispatch()
#4 /var/web/nginx/html/public/e.php(29): Phalcon\Mvc\Application-&gt;handle()
#5 {main}
  thrown in <b>/var/web/nginx/html/public/e.php</b> on line <b>8</b><br />

dispatch:beforeException never fires.

Phalcon was compiled using build/install script.

[ghost]

Please either build from ext/ or regenerate the files in build/ with php build/gen-build.php.

[temuri416]

yea, that was it. my exception logger is back to normal, thanks a lot.

could you please explain this ext/ rebuild business to me? why/when do I have to do it, when do fixes actually go into build/ ? what's the methodology.

[ghost]

Normally all bug fixes are committed to ext/ since this is what we use during development.

There is a special script, build/gen-build.php which generates combined sources in the build/ directory (this allows the compiler to apply more sophisticated optimizations because the entire code is gathered in the single file).

We (developers) run this script either before the release or when we change something drastically and want to make sure that the files in build/ can still be compiled.

Thus, to be on the safe side, I would recommend you to run build/gen-build.php when you are in doubt or build files from ext/.

When Phalcon build from ext/ it is usually a bit slower than Phalcon build from build/ but the performance penalty is really hard to notice on the developer machine :-)

[temuri416]

thank you so much for the explanation.

can I get your attention to #1078 - please see my latest comment.

[temuri416]

It looks like #1624 is now back. Can you please see if it could be somehow related to this bugfix?

Thanks!

[ghost]

No, not related

[gsouf]

What's about the exception handling with the event manager ?