Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error Handling misses catching Exceptions thrown inside controller actions #140

Closed
xblabs opened this issue Oct 23, 2012 · 5 comments
Closed

Comments

@xblabs
Copy link

xblabs commented Oct 23, 2012

With introducing error handling in the dispatcher here #66

there is still no way to handle errors occuring while processing an action. Lets assume my URI invokes IndexController::indexAction :

 public function indexAction()
 {

           throw new Exception( 'test' );
 }

Then this exception should be catched in the error handler to be forwarded to the errorAction in ErrorController to render a 500 error page for example. At the moment this is not working :

 $eventsManager->attach( 'dispatch', function( $e, $dispatcher, $exception ) {
            /** @var \Phalcon\Events\Event $e */
            $type = $e->getType();
            if( $type == 'beforeException' ) {
                if( $exception->getCode() == \Phalcon\Mvc\Dispatcher::EXCEPTION_HANDLER_NOT_FOUND) {
                    $action = 'error404';
               }else{
                    $action = 'error';
                }
                $dispatcher->forward(
                    array(
                         'controller' => 'error',
                         'action' => $action,
                         'params' => array( $exception )
                    )
                );
                return false;
            }
            return true;
        } );
@phalcon
Copy link
Collaborator

phalcon commented Oct 24, 2012

Unfortunately the Zend Engine does not provide a way to catch PHP exceptions in a C-extension, so currently it isn't possible to catch any kind of exception. We're doing research trying to simulate a try-catch.

Most Phalcon's users are using a set_exception_handler to solve this

@niden
Copy link
Member

niden commented Nov 7, 2012

Closing this issue since the zend engine cannot produce the result needed. set_exception_handler can be used to catch exceptions with a custom class.

@niden niden closed this as completed Nov 7, 2012
@ghost
Copy link

ghost commented Jul 16, 2013

How about the solution:

<?php

use Phalcon\Mvc\Dispatcher as PhalconDispatcher;
use Exception;

class Dispatcher extends PhalconDispatcher {

    /**
     * Dispatches a handle action taking into account the routing parameters
     * 
     * @return Phalcon\Mvc\Controller
     * @throws Exception
     */
    public function dispatch() {
        try {
            return parent::dispatch();
        } catch (Exception $exception) {
            $result = $this->getEventsManager()->fire('dispatch:beforeException', $this, $exception);

            if ($result === false) {
                return parent::dispatch();
            } else {
                throw $exception;
            }
        }
    }

}

@ghost
Copy link

ghost commented Jul 16, 2013

I think I have an idea how to implement this in C, will try to create a PoC code…

@phalcon
Copy link
Collaborator

phalcon commented Jul 16, 2013

@sjinks this is already implemented in 1.2.1, @mikemirten is posting it as example :)

@oh-ren
Copy link

oh-ren commented Jun 16, 2014

I'm just learning a bit more on this feat.

So it's intended that, when e.g. creating a Form with an invalid enitity (e.g. false), which throws an Exception, can be intercepted in the Dispatcher beforeException event?

One faulty bit, as it does seem to me, is for cases like the above the exception code is 0, which is the same as Dispatcher::EXCEPTION_NO_DI

This is faulty (as it has nothing to do with no DI available)?

Related, it also seems to me that the 2nd example of this chapter: http://docs.phalconphp.com/en/latest/reference/dispatching.html#handling-not-found-exceptions is a bit of a (very) bad example..
We can't just forward all other exceptions to a 503 page, and not all DispatchException's are related to 404..


(edit: I guess to fix the exception code issue, one must always check if the exception is an instance of DispatchException ?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants