Rethrowing exceptions from error handler

[alexwhitman]

I've got a couple of error handlers:

App::error(function(\Illuminate\Database\Eloquent\ModelNotFoundException $exception)
{
    // Both end up with an "Error in exception handler" error.
    //App::abort(404);
    //throw new Symfony\Component\HttpKernel\Exception\HttpException(404, null, $exception);
});

App::error(function(\Symfony\Component\HttpKernel\Exception\HttpException $exception)
{
    // I want to end up in here where I have generic http error handling code
});

Using MyModel::findOrFail($id) can throw the ModelNotFoundException which I'm catching but using App::abort or throwing a new exception from within the handler errors with an "Error in exception handler" error (from https://github.com/laravel/framework/blob/master/src/Illuminate/Exception/Handler.php#L318).

Is it not possible to re-throw an error in this way?

I can work around it by overriding findOrFail() in a base Model but I was wondering if the above could or should work?

[jasonlewis]

I think we spoke about this in IRC right?

But yeah, I just don't think that's how they're supposed to work. I'd just stick with overloading the findOrFail method for now. Simply overload it, use a try/catch to catch the ModelNotFoundException then throw the HttpException.

[alexwhitman]

Overloading findOrFail works, but then looks a bit strange in any instances where I want to catch the exception and not have it automatically 404:

try
{
    $user = User::findOrFail($id);
}
catch (HTTPException $e)
{
    // HTTP exception because a model instance couldn't be found? A bit odd.
}

[crynobone]

You should be catching ModelNotFoundException

try
{
    $user = User::findOrFail($id);
}
catch (Illuminate\Database\Eloquent\ModelNotFoundException $e)
{
    throw new Symfony\Component\HttpKernel\Exception\HttpException( ... );
}

Now wouldn't this nice if it was a Repository?

[alexwhitman]

I am catching ModelNotFoundException in my base model to then throw HTTPException as most of the time if the model isn't found I want to throw a 404. It just looks a bit odd catching a HTTPException outside of the base model in the few cases where I don't want to automatically 404. It's not a big issue but it feels like a bit of a workaround to the original issue where an Exception can't be thrown in App::error.

[crynobone]

It's not a big issue but it feels like a bit of a workaround to the original issue where an Exception can't be thrown in App::error.

Consider your use case and the current implementation on https://github.com/laravel/framework/blob/master/src/Illuminate/Exception/Handler.php#L219-L263.

Is it not possible to re-throw an error in this way?

As you can see from above code, it wasn't design to work that way.

[alexwhitman]

Consider your use case and the current implementation on https://github.com/laravel/framework/blob/master/src/Illuminate/Exception/Handler.php#L219-L263.

Looking at that code, if I change line 252 to $response = $this->callCustomHandlers($e, $fromConsole); then throwing an exception in the handler works.

[crynobone]

Yes but it not going to work consistently:

class FooException extends \Exception {}
class FoobarException extends FooException {}
class AwesomenessException extends \Exception {}

App::error(function(FooException $exception)
{
    throw new FoobarException;
});

App::error(function(FoobarException $exception)
{
    throw new AwesomenessException;
});

Route::get('foo', function()
{
    throw new FooException;
});

Symfony \ Component \ Debug \ Exception \ FatalErrorException
Maximum function nesting level of '100' reached, aborting!

class FooException extends \Exception {}
class FoobarException extends \Exception {}
class AwesomenessException extends \Exception {}

App::error(function(FooException $exception)
{
    throw new FoobarException;
});

App::error(function(FoobarException $exception)
{
    throw new AwesomenessException;
});

Route::get('foo', function()
{
    throw new FooException;
});

I would expect AwesomenessException to be thrown and logged to history (based on what we trying to archive with this changes) but instead:

[2013-07-26 16:13:23] log.ERROR: exception 'FooException' in /Users/crynobone/htdocs/playground/app/routes.php:30
Stack trace:
#0 [internal function]: {closure}()
#1 /Users/crynobone/htdocs/playground/vendor/laravel/framework/src/Illuminate/Routing/Route.php(80): call_user_func_array(Object(Closure), Array)
#2 /Users/crynobone/htdocs/playground/vendor/laravel/framework/src/Illuminate/Routing/Route.php(47): Illuminate\Routing\Route->callCallable()
#3 /Users/crynobone/htdocs/playground/vendor/laravel/framework/src/Illuminate/Routing/Router.php(1016): Illuminate\Routing\Route->run(Object(Illuminate\Http\Request))
#4 /Users/crynobone/htdocs/playground/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(531): Illuminate\Routing\Router->dispatch(Object(Illuminate\Http\Request))
#5 /Users/crynobone/htdocs/playground/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(506): Illuminate\Foundation\Application->dispatch(Object(Illuminate\Http\Request))
#6 /Users/crynobone/htdocs/playground/public/index.php(49): Illuminate\Foundation\Application->run()
#7 /Users/crynobone/htdocs/playground/server.php(19): require_once('/Users/crynobon...')
#8 {main} [] []

This just the one I can think of, but re-running the exception handlers array can easily odd results unless we really know what we are doing.

However being sad that, Laravel is easily extend to our need. The handler class can easily be replace and bind to the global $app container for such usage.

[alexwhitman]

I would expect AwesomenessException to be thrown and logged to history (based on what we trying to archive with this changes) but instead:

I've put in pull request #1959 which handles this and AwesomenessException is thrown/logged. It doesn't do anything about the nested recursion but I'll let Taylor give his opinion on the pull request before working around that as it's a bit more complex.

[taylorotwell]

I don't like what this opens up. We'll just have people complaining about WSODs because they forgot a handler, etc.