Guess this is PHP7 problem
problem is with the code
'function' => $nextframe['function'],
$nextframe doesn't necessary contain function index, it can be for example
array (
'file' => '/vagrant/dev/admin/vendor/composer/ClassLoader.php',
'line' => 412,
'include_filename' => '/vagrant/dev/admin/app/SaaS/Admin/Models/User.php',
'args' =>
array (
),
)
Addressing undefined index leads to error posted in subject.
I dont know the greatest way we could write a test for this (which I'd love to have, to make sure we don't fuck it up in the future), but the attached commit should resolve it and is safe with our upstream API.
I'm wondering if this is only for use statements, or if its also include/require functions, and if we should mock a function call as well.
I know it's an old issue, but I have the same issue on this line: https://github.com/getsentry/sentry-php/blob/master/src/Stacktrace.php#L119
Feel free to write a PR to fix this issue too. Also, I would like to remind you that we have a 2.0 beta release which is near its stable release, you should try it.
Sure, I'll submit a PR.
I'm already using 2.0.0-beta1. Is the master branch for 2.0 development?
Yes it is. I'm wondering in which case the function name isn't available in the frame though...
Maybe if it's a closure/lambda?
I don't think so because the function key at the line that is crashing is used only if class is available too... By the way there is for sure some strange case, but the fix should be as simple as adding an isset($frame['function])` here:
@pelmered are you able to create a reproducing case? I would like to port the resulting test to the master branch in case.
@Jean85 it happened to me too. I managed to reproduce it by using the Laravel framework version of the library, which in turn uses this, and it takes too long to return a response.
A case where there is a class index but not a function index is when the class for which the frame is generated is not executing a function but including / requiring a file.
A dump of what the $backtraceFrame variable contains:
/var/www/XXXX/vendor/sentry/sentry/src/Stacktrace.php:119:
array (size=6)
'type' => string '->' (length=2)
'class' => string 'Illuminate\View\Engines\CompilerEngine' (length=38)
'file' => string '/var/www/XXXX/vendor/laravel/framework/src/Illuminate/View/Engines/PhpEngine.php' (length=87)
'line' => int 43
'include_filename' => string '/var/www/XXXX/storage/framework/views/1ffb399f420225d5f568e4f4adb472013a7fb604.php' (length=89)
'args' =>
array (size=0)
empty
Contents of vendor/laravel/framework/src/Illuminate/View/Engines/PhpEngine.php around line 43:
<?php
namespace Illuminate\View\Engines;
// ....
class PhpEngine implements Engine
{
// ....
/**
* Get the evaluated contents of the view at the given path.
*
* @param string $__path
* @param array $__data
* @return string
*/
protected function evaluatePath($__path, $__data)
{
$obLevel = ob_get_level();
ob_start();
extract($__data, EXTR_SKIP);
// We'll evaluate the contents of the view inside a try/catch block so we can
// flush out any stray output that might get out before an error occurs or
// an exception is thrown. This prevents any partial views from leaking.
try {
include $__path; // line 43
} catch (Exception $e) {
$this->handleViewException($e, $obLevel);
} catch (Throwable $e) {
$this->handleViewException(new FatalThrowableError($e), $obLevel);
}
return ltrim(ob_get_clean());
}
Hopefully this helps solve the issue.
Composer versions:
Gotcha, it's an include. We need to add this case to our tests.
As far as I remember we already fixed the issue and added a test case, didn鈥檛 we?
Yes, but I think we should add this specific case to the tests.
I disagree since we have a test case (or we should have) that simply covers the fact that if the name of the function is not available then we don't break the app and this is the only thing that interest us. The reason about why such thing is not available is something that we don't care about, so we should not create such a specific test case. Also, this fix should be backported to 1.x if not already done
Closing this issue as it's pretty old. Should it still be present feel free to open a new issue
This doesn't appear resolved to me. In master at present, src/Stacktrace.php still isn't checking for the existence of a function key:
https://github.com/getsentry/sentry-php/blob/master/src/Stacktrace.php#L118
Converting that to:
if (isset($backtraceFrame['class']) && isset($backtraceFrame['function'])) {
appears to fix it for me. Happy to PR that if you would like, but I'm very unsure if the fix is legitimately that simple :)
Happy to receive any PR!
It would be helpful if you can reproduce the issue with a test though...
@jeff-h, it's not a conversion that is needed, but an addition. I will try to make a PR tonight with a test from my fork where I have it fixed.
The fix should be pretty easy, the test should include a new fixture (in tests/Fixtures) with the minimum stacktrace data to reproduce the issue and verify that the bug is fixed. It would be great if you could solve the issue for both 1.x and 2.x versions (so two PRs)
Created a PR for 2.x version.
Sorry to insist, but I'm totally unable to reproduce the issue and afaik @stayallive as well. It's not a problem to merge the fix of the referenced PR, but I would like to be sure that it's of some help. I cannot even find a include_filename key in any of the frames of the stacktraces I debugged, so the question is: are you using some extensions like XDebug or Blackfire that may be changing the stacktrace data? Otherwise, the best you can do to really help us is to post a little script reproducing the issue. This is what I've used:
class TestClass
{
public function includeFile()
{
try {
include __DIR__ . '/file-with-error.php';
} catch (Throwable $exception) {
var_dump($exception->getTrace());
}
}
}
(new TestClass())->includeFile();
where file-with-error.php contains:
<?php
foo
I am using XDebug on my local, but it also happens on instances without XDebug. I will try to find a specific case when it happens, but I also think that the code should check first if it has a key before using it.
We were able to finally reproduce it consistently and we are working on a fix for 2.1 on the linked PR
Any updates on this?
Yes, it has been fixed in the referenced PR and will be released with the next patch version
Most helpful comment
We were able to finally reproduce it consistently and we are working on a fix for
2.1on the linked PR