Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Support unknown format in LoggerDataCollector#22961
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| if (false !== ($pos =strpos($log,':')) &&$pos !== (strlen($log) -1)) { | ||
| list($pass,$message) =explode(':',$log,2); | ||
| }else { | ||
| $pass ='Unknown Compiler Pass'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what about something like:
$log =explode(':',$log,2);if (!isset($log[1]) || !class_exists($log[0],false)) {$log =array('Unknown Compiler Pass',implode(':',$log));}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nicolas-grekas this seems flaky due to disabling autoloading on the class_exists (becomes unknown). As this logging is aggregated during a run-time request, I can imagine the classes to never exist in the first place.
It works fine if I allow autoloading and I don't think this should be an issue for dev
| $logs =array(); | ||
| foreach (file($file,FILE_IGNORE_NEW_LINES)as$log) { | ||
| $log =explode(':',$log,2); | ||
| if (!isset($log[1]) || !class_exists($log[0])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
let's do it with a regexp instead of class_exists?|| !preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)++$/', $log[0])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That seems to work!
fabpot commentedMay 31, 2017
Thank you@iltar. |
… (iltar)This PR was merged into the 3.3 branch.Discussion----------[HttpKernel] Support unknown format in LoggerDataCollector| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#22952| License | MIT| Doc PR | ~The new expected format for the compiler log is `CompilerPassClass: Message`. However, this is not enforced by the old logging method as seen inschmittjoh/JMSDiExtraBundle#276This PR adds the ability to read those lines without crashing with `Uncaught Notice: Undefined offset: 1`.Please note that I have not tested this in an application so testers are welcome to confirm this fix!Commits-------a8dfbb1 Support unknown compiler log format
Uh oh!
There was an error while loading.Please reload this page.
The new expected format for the compiler log is
CompilerPassClass: Message. However, this is not enforced by the old logging method as seen inschmittjoh/JMSDiExtraBundle#276This PR adds the ability to read those lines without crashing with
Uncaught Notice: Undefined offset: 1.Please note that I have not tested this in an application so testers are welcome to confirm this fix!