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

No stack when ErrorException is thrown by Laravel #424

Closed
carlosvini opened this issue Jul 1, 2016 · 6 comments
Closed

No stack when ErrorException is thrown by Laravel #424

carlosvini opened this issue Jul 1, 2016 · 6 comments

Comments

@carlosvini
Copy link

Laravel throws ErrorException in some places like errors in views. Whoops threats ErrorException as if they all were being thrown by a shutdown handler:

https://github.com/filp/whoops/blob/master/src/Whoops/Exception/Inspector.php#L168

I think at this point (PHP 7) most errors are catchable with a custom error handler, while others still need a shutdown handler.

Maybe changing the code to something like this would be a good compromise between error handler/shutdown handler:

if (!$e instanceof \ErrorException || !extension_loaded('xdebug') || !xdebug_is_enabled()) {
        return $traces;
}

Instead of the current code which returns an empty stack:

if (!extension_loaded('xdebug') || !xdebug_is_enabled()) {
    return array();
}

Thx!

@denis-sokolov
Copy link
Collaborator

But the code at your link looks exactly as you mentioned. It does not look as you claim. Perhaps you had an old version of Whoops? Let us know if you have new information.

@carlosvini
Copy link
Author

Thx for your reply. My only problem is that it returns an empty array at this line:
https://github.com/filp/whoops/blob/master/src/Whoops/Exception/Inspector.php#L177

  if (!extension_loaded('xdebug') || !xdebug_is_enabled()) {
      return [];
  }

I don't know what was the initial reasoning for this but I think it's weird for a library that's supposed to make debug easier to show nothing when there's an error, just because someone doesn't have xdebug installed.

Maybe change it to:

  if (!extension_loaded('xdebug') || !xdebug_is_enabled()) {
      return $traces;
  }

@denis-sokolov
Copy link
Collaborator

The code is, admittedly, a bit confusing.

Note that that return statement is only hit when the severity of the error is fatal. Which means we got here from a shutdown handler. In those cases the traces regular PHP provides are useless, as they only go until the shutdown handler, not to the actual error.

Does Laravel throw error exceptions with fatal severity errors? That is confusing.

@carlosvini
Copy link
Author

carlosvini commented Sep 5, 2016

is only hit when the severity of the error is fatal

if (!Misc::isLevelFatal($e->getSeverity())) {

It is only hit when the severity is NOT fatal. Notice the ! before isLevelFatal

I will try to add more context when i have time.

Thx.

@denis-sokolov
Copy link
Collaborator

Well, yes, but in that condition we do return the traces:

if (!Misc::isLevelFatal($e->getSeverity())) {
    return $traces;
}
if (!extension_loaded('xdebug') || !xdebug_is_enabled()) {
    return [];
}

@carlosvini
Copy link
Author

carlosvini commented Sep 5, 2016

You're right, sorry. That's what happens when you try to reply in the middle of other tasks.
I will try to add more context later.

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

2 participants