json_encode recursion detected warning #137
Conversation
|
This code is wrong as it will never log the context |
|
A proper fix would be to handle recursion in the NormalizerFormatter |
|
Yeah the problem is that whatever is passed in the context array has a recursive reference somewhere and that blows up json_encode. Handling recursion is gonna suck and make it slower though, but I guess we have no choice. |
|
I think that @schmittjoh's serializer handles that case, we may take inspiration from it or even use it ? |
|
@benja-M-1 the serializer is a bit overbloated/overkill for the use case of logging IMO. This should be as fast as possible ideally. I wish json_encode had a flag to ignore recursion. |
|
Yes I do agree it's an overkill. I will try to do something today |
|
Actually.. json_encode simply triggers a warning and then returns null where the recursion occurred. So it's maybe enough to do a nasty @json_encode or to register a custom error handler (not sure what's the fastest). It could be done only when the context/extra keys aren't empty to avoid slowing down normal messages. |
| } | ||
| if (count($message) === 1) { | ||
| $message = reset($message); | ||
| } | ||
|
|
||
| // Handle json_encode recursion error | ||
| if ($handleError) { | ||
| set_error_handler(function () { }); |
There was a problem hiding this comment.
I'm not sure what to do here. Should it raise a new error if it's the json_encode that raised the error first?
|
@Seldaek do I have something else to do? |
|
Nope, merged and refactored things a bit so that it's applied to all formatters and not just the wildfire case. |
|
When do you plan to create a new tag with, at least, this fix? |
|
This week sometime. I'm trying to merge just a couple more PRs and then release. |
|
Great, thank you! Keep up the good work :) |
Fixes symfony/symfony#6427