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

NormalizerFormatter skips some stacktraces #1736

Open
WoZ opened this issue Jul 19, 2022 · 5 comments
Open

NormalizerFormatter skips some stacktraces #1736

WoZ opened this issue Jul 19, 2022 · 5 comments
Labels

Comments

@WoZ
Copy link
Contributor

WoZ commented Jul 19, 2022

Monolog version 2|3

Check the function.

$trace = $e->getTrace();
foreach ($trace as $frame) {
if (isset($frame['file'])) {
$data['trace'][] = $frame['file'].':'.$frame['line'];
}
}

PHP Fatal Error caught by register_shutdown_function() has trace field. Inside this trace field there can be few elements without file and line fields.

Here is an example:

# php test.php 
PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in /var/www/html/test.php on line 27
array(1) {
  [0]=>
  array(1) {
    ["function"]=>
    string(16) "shutdownFunction"
  }
}

Run this script and you will see the following output of the $e->getTrace() method:

You may change register(false) to register(true) and check the results:

# php test.php 
PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in /var/www/html/test.php on line 28
array(2) {
  [0]=>
  array(3) {
    ["file"]=>
    string(22) "/var/www/html/test.php"
    ["line"]=>
    int(11)
    ["function"]=>
    string(16) "shutdownFunction"
  }
  [1]=>
  array(3) {
    ["function"]=>
    string(14) "shutdownMethod"
    ["class"]=>
    string(12) "ErrorHandler"
    ["type"]=>
    string(2) "::"
  }
}

All that records without file and line will be skipped and engineer can't understand the full picture.

If you agree with this I may provide PR.

@WoZ WoZ added the Bug label Jul 19, 2022
WoZ added a commit to BETER-CO/yii2-beter-logging that referenced this issue Jul 20, 2022
…cktrace. This causes Monolog\Formatter\NormalizerFormatter to return normalized error without "trace" field at all. But code expects it. Fix of this behavior. Check monolog related issue - Seldaek/monolog#1736
WoZ added a commit to BETER-CO/yii2-beter-logging that referenced this issue Jul 20, 2022
…cktrace. This causes Monolog\Formatter\NormalizerFormatter to return normalized error without "trace" field at all. But code expects it. Fix of this behavior. Check monolog related issue - Seldaek/monolog#1736
@Seldaek
Copy link
Owner

Seldaek commented Jul 22, 2022

I would rather not change the format, right now it normalizes to an array of file/line arrays. If you think you can fix it without changing the data type then please send a PR (or tell me how if you'd rather discuss first).

@WoZ
Copy link
Contributor Author

WoZ commented Jul 23, 2022

@Seldaek I agree with you in general. I've found that sentry does the following with such errors.

#3 /app/main/Yii.php(62): Yii::error
#2 /app/vendor/yiisoft/yii2/base/ErrorHandler.php(318): yii\base\ErrorHandler::logException
#1 /app/vendor/yiisoft/yii2/base/ErrorHandler.php(283): yii\base\ErrorHandler::handleFatalError
#0 [internal](0): null

So, we may assume that someone parses an array for file/line string elements. The only option is to use a semicolon as a delimiter. So, the option is to mimic this format not to break compatibility.

Option 1. [internal]:0, as sentry does
Option 2. internal[shutdownFunction]:0 and internal[ErrorHandler][shutdownMethod]:0
Option 3. internal(shutdownFunction):0 and internal[ErrorHandler](shutdownMethod):0

P.S. I missed to attach my test.php script.

<?php

function shutdownFunction() {
    $e = error_get_last();
    $exception = new \ErrorException($e['message'], $e['type'], $e['type'], $e['file'], $e['line']);
    var_dump($exception->getTrace());
}

class ErrorHandler {
    public static function shutdownMethod() {
        shutdownFunction();
    }
}

function register($classOrNot = false) {
    if ($classOrNot) {
        register_shutdown_function([ErrorHandler::class, 'shutdownMethod']);
    } else {
        register_shutdown_function('shutdownFunction');
    }
}

register(false);

$buffer = [];
while (true) {
    $buffer[] = str_repeat('+', 1024);
}

@Seldaek
Copy link
Owner

Seldaek commented Jul 23, 2022

Id maybe do this as option 4 for methods but otherwise yes I like this idea:

internal[ErrorHandler->shutdownMethod]:0

@WoZ
Copy link
Contributor Author

WoZ commented Jul 23, 2022 via email

@Seldaek
Copy link
Owner

Seldaek commented Jul 23, 2022

Sure, the point was just to avoid : :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants