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

Can't catch exception in middleware #14573

Closed
rudiedirkx opened this issue Jul 31, 2016 · 14 comments
Closed

Can't catch exception in middleware #14573

rudiedirkx opened this issue Jul 31, 2016 · 14 comments

Comments

@rudiedirkx
Copy link

@rudiedirkx rudiedirkx commented Jul 31, 2016

I want a middleware package to catch an exception in its middleware.

  • Exception: FormValidationException
    • Thrown by a validator helper, or app logic, in the controller
  • Middleware: HandleFormValidation
    • Catches only that exception and does a redirect with data & errors etc

But the exception can't be caught by the middleware, because it's already been caught and handled somewhere else. The $response object ($next() return value) is an exception error page.

Who catches my exception, and where, and why so early? Aren't exceptions allowed in the middleware pipeline?

The app can extend its App\Exceptions\Handler to 'render' that exception into a redirect, but I want all logic to be in the package, not just half.

Middleware looks like this:

public function handle(Request $request, Closure $next) {
  try {
    return $next($request); // Passes through an uncaught exception error page
  }
  catch (\Exception $ex) {
    exit(__METHOD__); // Never gets here
  }
}
@GrahamCampbell

This comment has been minimized.

Copy link
Member

@GrahamCampbell GrahamCampbell commented Jul 31, 2016

Yes, this is the beavhiour starting from L5.2. Throwing an exception causes the response to be set as that returned from the exception handler, and then the middleware is allowed to backout from that point.

@rudiedirkx

This comment has been minimized.

Copy link
Author

@rudiedirkx rudiedirkx commented Jul 31, 2016

Back out from that point how? Where is my exception? How can non-app code do something with specific exceptions? Do I really extract it from the response object? That seems silly...

@GrahamCampbell

This comment has been minimized.

Copy link
Member

@GrahamCampbell GrahamCampbell commented Jul 31, 2016

We don't recommend you write try catch blocks in middleware. Instead, handle exceptions inside the exception handler. Perhaps https://github.com/GrahamCampbell/Laravel-Exceptions would be of use to you?

@rudiedirkx

This comment has been minimized.

Copy link
Author

@rudiedirkx rudiedirkx commented Jul 31, 2016

Yeah, I don't like that, because only 1 party can extend a base class. What if 4 packages want specific exception handling? That's why middleware is perfect.

This does the trick, but it feels really funky:

public function handle(Request $request, Closure $next) {
  $response = $next($request);

  // 'Catch' our FormValidationException and redirect back.
  if (!empty($response->exception) && $response->exception instanceof FormValidationException) {
    return redirect()->back()->withErrors($response->exception->form->getErrors())->withInput();
  }

  return $response;
}

Is that what you recommend? It can't be in App\Exceptions\Handler, because that's app logic.

Thanks for super fast replies!

I'm still curious how this handling works though. I was trying to debug the middleware pipeline, but I couldn't find any exception catching. I really thought my middleware was the first to catch it. Where does that happen?

@GrahamCampbell

This comment has been minimized.

Copy link
Member

@GrahamCampbell GrahamCampbell commented Jul 31, 2016

I guess it would be conventional for the application to still handle package exceptions. Feel free to ask around on the forums for solutions other people might have.

In terms of where does this happen, it happens in our pipeline. Starting with L5.2, there's a custom http pipeline: https://github.com/laravel/framework/blob/5.2/src/Illuminate/Routing/Pipeline.php.

@GrahamCampbell

This comment has been minimized.

Copy link
Member

@GrahamCampbell GrahamCampbell commented Jul 31, 2016

Your observation about accessing the exception on the response could be useful for package exception handling, but I can't say it's something I've had to do before (hence why I'd recommend asking on the forums to find someone who might have tried to do this).

@ivorobioff

This comment has been minimized.

Copy link

@ivorobioff ivorobioff commented Jul 23, 2017

What a crappy framework!!!

@pablorsk

This comment has been minimized.

Copy link
Contributor

@pablorsk pablorsk commented Oct 5, 2017

Hi! The correct way is don't catch errors directly on middleware. You need add a custom ExceptionHandler. If you like register handler on your middleware, you can do it, but forget try/catch.

How catch errors without touching App\Exceptions\Handler file:

Register your CustomExceptionHandler

/* @var ExceptionHandler Illuminate\Contracts\Debug\ExceptionHandler */
$previousHandler = null;
if (app()->bound(ExceptionHandler::class) === true) {
    $previousHandler = app()->make(ExceptionHandler::class);
}
app()->singleton(ExceptionHandler::class, function () use ($previousHandler) {
    return new CustomExceptionHandler($previousHandler);
});

And your basic CustomExceptionHandler

class CustomExceptionHandler implements ExceptionHandlerInterface
{
    /**
     * @var ExceptionHandlerInterface|null
     */
    private $previous;

    public function __construct(ExceptionHandlerInterface $previous = null)
    {
        $this->previous = $previous;
    }

    public function report(Exception $exception)
    {
        $this->previous === null ?: $this->previous->report($exception);
    }

    public function render($request, Exception $exception)
    {
        if ($exception instanceof CustomExceptionHandler) {
            echo 'This is my particular way to show my errors';
        } else {
            $response = $this->previous === null ? null : $this->previous->render($request, $exception);
        }

        return $response;
    }

    /**
     * {@inheritdoc}
     */
    public function renderForConsole($output, Exception $exception)
    {
        /* @var OutputInterface $output */
        $this->previous === null ?: $this->previous->renderForConsole($output, $exception);
    }
}
@kmuenkel

This comment has been minimized.

Copy link

@kmuenkel kmuenkel commented Nov 2, 2017

I'm pretty sure Barryvdh\Debugbar does something like this. I just encountered a scenario where a middleware was bombing out, and failing quietly. No errors in the log, and only a vague 500 error response. Then I noticed that only Debugbar was detecting what was actually wrong.

It looks like the package includes its own Middleware\Debugbar class designed to handle middleware exceptions, a similar tactic to what @pablorsk was describing. I'm dissecting it at the moment to see if it can offer any clues to how I might write my own handler that can exist outside of debug-mode. I've gotten as far as identifying the try/catch in Debugbar::handle as a red herring. That middleware exceptions end up in the $response->exceptions, as stated earlier, though I'm not certain how yet. They begin getting handled in Barryvdh\Debugbar\LaravelDebugbar::modifyResponse().

Beyond that, tracing gets a little tricky, since the data gets buried in associative arrays and inherited class properties, rather than method responses. And pablorsk's answer may spare me having to do that. But @rudiedirkx, you may want to have a look. Or, if you only need this for development, just install that package and don't worry about it.

@wojcikm

This comment has been minimized.

Copy link

@wojcikm wojcikm commented Nov 2, 2017

What about the situation, when some small part of application goes through this middleware and I don't wan't to handle it global handler? Is it possible to disable this pipeline?

@kmuenkel

This comment has been minimized.

Copy link

@kmuenkel kmuenkel commented Nov 2, 2017

@warlof - Well you don't have to register all middleware in the Kernel. You could apply it with the Illuminate\Routing\Controller::middleware() method, meaning you could wrap some logic around when to apply it. You could also manage the behavior therein with some configuration variables, maybe derived from environment ones, and even alter those config values within your app (though that feels a little sloppy).

There's also a 'middleware.disable' flag that you can leverage, but that's kind of all-or-nothing thing, usually used for unit tests. I'm not entirely certain how to use it outside of the PhpUnit WithoutMiddleware trait, but you could take a look at how that works, and the Illuminate\Routing\Router::runRouteWithinStack() method that actually looks at the flag.

You might also consider writing an override for the Illuminate\Routing\Router::gatherRouteMiddleware() method. Maybe something that could look to a config array of 'allowable' middlewares, as it compiles what ones are assigned to the given route.

@topoff

This comment has been minimized.

Copy link

@topoff topoff commented Dec 8, 2017

I'm developing a package for user tracking. Its called from a middleware, but if anything fails, I want to just log the error and go further, that the user doesn't recognize anything about that, because it's not app critical. I want to do that in the package itself, not in the application. Isn't that possible to achieve?

@kmuenkel

This comment has been minimized.

Copy link

@kmuenkel kmuenkel commented Dec 8, 2017

The trouble is, I think Middleware follows the Chain of Responsibility pattern, similar to a Handler-Stack, by way of a series of nested Closures. I'm not really sure how to interrupt that flow, given that you can't exactly write an override class for a Closure that could try-catch the error, and more importantly, have an awareness of what the next link in the chain is. Middleware classes don't extend a common base-class for you to leverage either. So the only thing I could think of would be to take a step further back from that, to how Middleware gets stored and executed in the first place. Have something that wraps each closure in a closure of your own that's designed to handle the errors and then move on to the next one.

Current flow:

______________________       ______________________
|Middleware Closure   |  ->  |Middleware Closure   | 
|_____________________|      |_____________________|

Your flow:

_______________________________       _______________________________
|Your Error-Handling Closure   |  ->  |Your Error-Handling Closure   |
|  ______________________      |      |  ______________________      |
|  |Middleware Closure   |     |      |  |Middleware Closure   |     |
|  |_____________________|     |      |  |_____________________|     |
|______________________________|      |______________________________|

I'm not really sure where what class you'd have to override to do this. I think I'd start by dissecting the Kernel, or using debug_backtrace inside of one of the Middleware classes to see what fires it. Best of luck. And please let me know how it turns out.

@hellfull

This comment has been minimized.

Copy link

@hellfull hellfull commented Jul 18, 2018

@ivorobioff I will be happy to work with your "not crappy" framework anytime soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.