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

Bugfix/stacktrace is missing for haltable exceptions #524

Conversation

Vmak11
Copy link
Contributor

@Vmak11 Vmak11 commented Aug 22, 2017

I noticed I was missing a stack trace for a FatalThrowableError Exception that should have had one and found it was caused by returning an empty array for the stack trace due to a commit adding ability to get stack trace from xdebug for fatal errors:
c7ab118

* @return array
*/
protected function getTrace($e)
protected function getTrace(\Throwable $e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cant use \Throwable her it is only available in php 7.0

@Vmak11 Vmak11 force-pushed the bugfix/stacktrace-is-missing-for-haltable-exceptions branch from f4d733a to 263ad8a Compare August 22, 2017 17:52
@staabm
Copy link
Contributor

staabm commented Aug 22, 2017

Could you add a unit test?

@Vmak11 Vmak11 force-pushed the bugfix/stacktrace-is-missing-for-haltable-exceptions branch from 263ad8a to 2861a7e Compare August 22, 2017 19:32
@Vmak11
Copy link
Contributor Author

Vmak11 commented Aug 23, 2017

@staabm

Do you have any suggestions about how to unit test this as I'm not very familiar with Xdebug as well as setting up unit tests that require extensions to be installed.

@staabm
Copy link
Contributor

staabm commented Aug 24, 2017

Add a unit test which reproduces the issue beeing fixed, add a "skip-condition" to not run it when xdebug is not loaded and assert via try/catch that the thrown exception will have a proper Stacktrace when provided to getInspector()[1]

[1] https://github.com/filp/whoops/blob/5c1d43f63f26599de3a07253a371db374752b006/docs/API%20Documentation.md#methods

@denis-sokolov
Copy link
Collaborator

Would the following change achieve the same goal?

https://github.com/filp/whoops/compare/get-trace-fix-524

@denis-sokolov
Copy link
Collaborator

Hopefully fixed in 51c2de8.

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

Successfully merging this pull request may close these issues.

None yet

5 participants