laravel / lumen-framework

The Laravel Lumen Framework.
https://lumen.laravel.com
MIT License
1.48k stars 419 forks source link

Fatal exceptions are handled twice #657

Closed tobiassjosten closed 6 years ago

tobiassjosten commented 7 years ago

Description:

When a controller throws a fatal exception it'll be sent to and reported/rendered by the exception handler twice, duplicating the output.

In my example I have a CrashController which triggers a SoapFault. It's sent to my exception Handler which renders an "i".

Expected output: i Actual output: ii

This also duplicates any logging you might have in the report() method of your handler.

Steps To Reproduce:

See lumen-exception-bug for an example.

themsaid commented 7 years ago
screen shot 2017-09-25 at 8 40 04 pm screen shot 2017-09-25 at 8 40 11 pm
tobiassjosten commented 7 years ago

@themsaid Are you using PHP's built-in webserver? php -S '127.0.0.1:8000' -t public

themsaid commented 7 years ago

@tobiassjosten nope I'm running on laravel valet.

tobiassjosten commented 7 years ago

@themsaid Sorry, I was being vague – could you try PHP's built-in webserver and see if you get the bug then? That's what I'm using locally when it hits me.

Also, are you on PHP 7.0 or later?

tobiassjosten commented 7 years ago

Here's another example of this error, @themsaid: https://github.com/tobiassjosten/lumen-exception-bug/commit/b29dfdd54716c3854e6476239f203273f61bb12e

When visiting that new controller you'd expect to get a "x" response but instead you get a "xi" response. This is because the registered shutdown function kicks in, detects an "uncaught exception", and thus calls the exception handler to create a response for it (the "i" part).

The problem here is that new \SoapClient('asdf') (and others, I'm sure) triggers an error that seems to be treated differently to exceptions by PHP. Even if they're caught and handled they still remains in error_get_last() and so the shutdown handler does its thing.

With this new information I agree my PR wasn't the best approach. I'd instead suggest to target the shutdown function:

Let me know what you think. I'd be happy to roll a new PR!

driesvints commented 6 years ago

Closing this issue because it's already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue.