fix for #4614 breaks error handler using Zend\Log #5383
Conversation
|
I fail to see how #4616 is related to #2330; they do not touch the same sets of code at all. Please provide:
and test with the code as it was when 2330 was committed to verify it works. If it does, use git bisect from that commit to head to determine the actual location where the bug was reintroduced. We can discuss solutions at that point. |
|
We are working with ZendSkeletonApplication and 2330 is too old for that to work without big changes so I prepared fork in https://github.com/jkavalik/ZendSkeletonApplication with included logger and registered default logger error handler, and three notices generated in default index
expected results
actual results
|
|
Can you write this as a unit test for us? |
|
Not sure but I can try :) |
|
Ok, I think I did - jkavalik/zf2@f121d22 |
|
The test does indeed fail. Removing the |
- The error handling calls pop the scope when already inside an error handler -- such as when you use the Logger as an error handler. The only call inside the block that could be a potential problem is the one to format a DateTime object -- and then, only if the format is invalid (in which case you have bigger problems).
|
Turns out the issue is with recursion, which the error handling was taking care of. Checking into alternate ways to deal with this. |
- The only viable solution that (a) prevents recursion during serialization, and (b) allows the Logger to work as an error handler after a message has been formatted, is to use the error suppression operator. See the comment in the normalize method for details.
|
Had to use error suppression; it is the only way for an error handler to suppress errors properly within the body of its work. Using another error handler does not work in this scope, as it essentially removes the current error handler when you pop it off the stack. |
|
I tried to find some recursion detection, but only which seem working is taking print_r of array and searching for "RECURSION", which would be probably performance killer and imho as ugly as error supression |
|
Test failure is on 5.5, and appears to be an unrelated segfault (log tests pass). |
|
@jkavalik any detection of recursion in this context is going to be bad on performance. The original solution was fine (ignore the error); the problem is it didn't take into account that it could be nested inside another error handler. Ugly situation. |
|
@weierophinney is there any possibility for that recursive array to come from error handler logging? if not, we could turn those start/stop off inside error handling (but I'm not sure if we can detect that it IS error handling) |
|
@jkavalik too difficult to manage -- it requires the formatter know the context in which it is called, which violates a whole slough of design principals. |
|
@weierophinney I was afraid of that. Would be nice to temporarily change formatter on the fly.. but again depending on some state.. |
|
Hi, I have the same Problem on the current dev-master. After the first logged error, the logger is disabled. I need the error logger for storing the errormessages to integrate them in my layout. I found a workaround but I am not sure if there are side effects that cause errors in the future. Instead of using Zend\Log\Writer\AbstractWriter as a parentclass for the ErrorWriter, I create a own class that implements Zend\Log\Writer\WriterInterface with no other functionality, than storing the messages. see this: http://pastebin.com/0XxPdT1M Any comments on that? |
|
@weierophinney @jkavalik I really don't understand why this failure occurs but the JSON formatting is only to generate a good readable error message as a one-liner. It's not required to be valid JSON - it's required to be human readable, only. So in my opinion it makes sense to format with PS: another issue with JSON formatting is the defined utf-8 encoding of the JSON spec were the logger doesn't define such encoding. |
|
@weierophinney Please rebase the PR |
- The error handling calls pop the scope when already inside an error handler -- such as when you use the Logger as an error handler. The only call inside the block that could be a potential problem is the one to format a DateTime object -- and then, only if the format is invalid (in which case you have bigger problems).
- The only viable solution that (a) prevents recursion during serialization, and (b) allows the Logger to work as an error handler after a message has been formatted, is to use the error suppression operator. See the comment in the normalize method for details.
|
@Maks3w rebased. |
- The error handling calls pop the scope when already inside an error handler -- such as when you use the Logger as an error handler. The only call inside the block that could be a potential problem is the one to format a DateTime object -- and then, only if the format is invalid (in which case you have bigger problems).
- The only viable solution that (a) prevents recursion during serialization, and (b) allows the Logger to work as an error handler after a message has been formatted, is to use the error suppression operator. See the comment in the normalize method for details.
In #4616 problem was reintroduced with calling Zend\Stdlib\ErroHandler::start/stop() one level deeper than it was removed from in #2330 - when we tried to use default Zend\Log\Logger::registerErrorHandler() it only logs first php error.
We are using LogWriterStream and after some testing and searching even with $stream_writer->setConvertWriteErrorsToExceptions(false); but tracking showed, that another calls to Zend\Stdlib\ErrorHandler occurs in formatter which was added later.