[
https://issues.apache.org/jira/browse/LOG4J2-3286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17464998#comment-17464998
]
Ben Thurley commented on LOG4J2-3286:
-------------------------------------
[~ckozak]
Thanks for looking at this, I can see the change now, I missed it before
because I wasn't sure which branch to look at. I guess this is on the
release-2.x branch?
It looks like this code has swapped a few times between catching Exception or
Throwable.
The problem we have is a custom appender used for unit testing that verifies
log output. It is throwing java.lang.AssertionError under certain conditions
which until now we could see in our unit tests. Since 2.17 these errors are
swallowed and wrapped with a log4j LoggingException which doesn't match the
AssertionError and so a number of tests are now failing.
Honestly I'm not that enamoured with our unit tests being reliant on a 3rd
party library working in a specific way. The way this is setup with a custom
appender though it will easily apply to all tests, whereas if a mock was used
then each test would need extra setup.
In any case, isn't catching Throwable considered a bad practice? Errors are
meant to be unrecoverable situations like OutOfMemory or StackOverflow, or
indeed AssertionError.
>From the JavaDoc:
"An {{Error}} is a subclass of {{Throwable}} that indicates serious problems
that a {+}reasonable application should not try to catch{+}. Most such errors
are abnormal conditions."
Yet any application using log4j is now inadvertently catching these errors.
> 2.17 has reverted LOG4J-2972 fix in AppenderControl but change not in history
> -----------------------------------------------------------------------------
>
> Key: LOG4J2-3286
> URL: https://issues.apache.org/jira/browse/LOG4J2-3286
> Project: Log4j 2
> Issue Type: Question
> Components: Appenders
> Affects Versions: 2.17.0
> Environment: Windows desktop running unit tests with log4j 2.17
> Reporter: Ben Thurley
> Priority: Major
>
> Upgrading to 2.17 to fix the slew of vulnerabilities has resulted in a change
> to error handling in the logs.
> Specifically in the class:
> org.apache.logging.log4j.core.config.AppenderControl
> There is a method called tryCallAppender() which as of 2.14.1 had been set to
> catch Exception. Looking at the history I can see no further changes to
> revert this to handling Throwable, however, comparing the source for 2.16.
> and 2.17 this is exactly what has changed.
> I don't believe this change should have reverted and if it should then why
> isn't there a commit for it in the history?
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
