This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch fix/2.x/handles-throwable in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 433fe04e4b8de7f190ace3d8b986bb5f0edd5346 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Sun Sep 7 23:13:35 2025 +0200 fix: recognize nested converters in `alwaysWriteExceptions` `PatternParser` was not detecting when a throwable was already rendered inside a *nested* converter (e.g. `%style{%ex}{red}`). Because `handlesThrowable` was missing for those wrappers, the parser assumed no throwable was present and incorrectly auto-appended another `%ex`, causing the exception to be logged twice. This change implements `handlesThrowable` for the relevant nested converters so that `alwaysWriteExceptions` correctly recognizes them and no duplicate stack traces are produced. --- .../log4j/core/pattern/PatternParserTest.java | 51 +++++++++++++++++++++- .../core/pattern/AbstractStyleNameConverter.java | 7 +++ .../core/pattern/EncodingPatternConverter.java | 7 ++- .../pattern/EqualsBaseReplacementConverter.java | 7 +++ .../log4j/core/pattern/HighlightConverter.java | 9 ++-- .../core/pattern/LogEventPatternConverter.java | 11 +++-- .../log4j/core/pattern/MaxLengthConverter.java | 7 +++ .../core/pattern/RegexReplacementConverter.java | 7 +++ .../logging/log4j/core/pattern/StyleConverter.java | 9 ++-- .../VariablesNotEmptyReplacementConverter.java | 7 +++ src/changelog/.2.x.x/3920-nested-throwables.xml | 12 +++++ 11 files changed, 112 insertions(+), 22 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java index f493e25bb6..53602d9ea2 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/PatternParserTest.java @@ -16,6 +16,7 @@ */ package org.apache.logging.log4j.core.pattern; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -40,6 +41,8 @@ import org.apache.logging.log4j.util.StringMap; import org.apache.logging.log4j.util.Strings; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class PatternParserTest { @@ -89,6 +92,50 @@ class PatternParserTest { validateConverter(formatters, 1, "Line Sep"); } + @ParameterizedTest + @ValueSource(strings = {"%m", "%p", "%c", "%d"}) + void testAlwaysWriteExceptions_appendsThrowable(String pattern) { + // With alwaysWriteExceptions=true, parser should auto-append a %throwable + final List<PatternFormatter> formatters = parser.parse(pattern + "%n", true, false, false); + assertThat(formatters).hasSize(3); + assertThat(formatters.get(2).getConverter()).isInstanceOf(ThrowablePatternConverter.class); + + // With alwaysWriteExceptions=false, parser should leave the pattern unchanged + final List<PatternFormatter> formatters2 = parser.parse(pattern + "%n", false, false, false); + assertThat(formatters2).hasSize(2); + assertThat(formatters2.get(1).getConverter()).isNotInstanceOf(ThrowablePatternConverter.class); + } + + @ParameterizedTest + @ValueSource( + strings = { + "%black{%throwable}", + "%blue{%throwable}", + "%cyan{%throwable}", + "%green{%throwable}", + "%magenta{%throwable}", + "%red{%throwable}", + "%white{%throwable}", + "%yellow{%throwable}", + "%encode{%throwable}{JSON}", + "%equals{%throwable}{java.lang.Throwable}{T}", + "%equalsIgnoreCase{%throwable}{java.lang.Throwable}{T}", + "%highlight{%throwable}", + "%maxLen{%throwable}{1024}", + "%replace{%throwable}{\n}{ }", + "%style{%throwable}{red bold}", + "%notEmpty{%throwable}", + }) + void testAlwaysWriteExceptions_recognizesNestedPatterns(String pattern) { + // With alwaysWriteExceptions=true, parser must detect the nested %throwable + // and NOT auto-append another one at the top level + final List<PatternFormatter> formatters = parser.parse(pattern, true, false, false); + + // Only one top-level formatter is expected (the wrapper itself), not a trailing ThrowablePatternConverter + assertThat(formatters).hasSize(1); + assertThat(formatters.get(0).getConverter()).isNotInstanceOf(ThrowablePatternConverter.class); + } + /** * Test the custom pattern */ @@ -116,8 +163,8 @@ class PatternParserTest { formatter.format(event, buf); } final String str = buf.toString(); - final String expected = "INFO [PatternParserTest :101 ] - Hello, world" + Strings.LINE_SEPARATOR; - assertTrue(str.endsWith(expected), "Expected to end with: " + expected + ". Actual: " + str); + final String expected = "INFO [PatternParserTest :148 ] - Hello, world" + Strings.LINE_SEPARATOR; + assertThat(str).endsWith(expected); } @Test diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AbstractStyleNameConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AbstractStyleNameConverter.java index b7b5e35018..98d69acade 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AbstractStyleNameConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AbstractStyleNameConverter.java @@ -376,4 +376,11 @@ public abstract class AbstractStyleNameConverter extends LogEventPatternConverte toAppendTo.append(AnsiEscape.getDefaultStyle()); } } + + @Override + public boolean handlesThrowable() { + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EncodingPatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EncodingPatternConverter.java index 970517f69f..2458e2cb70 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EncodingPatternConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EncodingPatternConverter.java @@ -54,10 +54,9 @@ public final class EncodingPatternConverter extends LogEventPatternConverter { @Override public boolean handlesThrowable() { - return formatters != null - && formatters.stream() - .map(PatternFormatter::getConverter) - .anyMatch(LogEventPatternConverter::handlesThrowable); + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); } /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsBaseReplacementConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsBaseReplacementConverter.java index 904d498169..e71edb6147 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsBaseReplacementConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/EqualsBaseReplacementConverter.java @@ -99,4 +99,11 @@ public abstract class EqualsBaseReplacementConverter extends LogEventPatternConv substitutionBuffer.append(substitution); } } + + @Override + public boolean handlesThrowable() { + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java index 29663b4f5a..6f72d2f47e 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java @@ -265,11 +265,8 @@ public final class HighlightConverter extends LogEventPatternConverter implement @Override public boolean handlesThrowable() { - for (final PatternFormatter formatter : patternFormatters) { - if (formatter.handlesThrowable()) { - return true; - } - } - return false; + return patternFormatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LogEventPatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LogEventPatternConverter.java index 4d4bc2cbc7..4c175abdb4 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LogEventPatternConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/LogEventPatternConverter.java @@ -53,13 +53,16 @@ public abstract class LogEventPatternConverter extends AbstractPatternConverter } /** - * Normally pattern converters are not meant to handle Exceptions although few pattern converters might. + * Tests whether this pattern converter is renders a {@link Throwable}. + * * <p> - * By examining the return values for this method, the containing layout will determine whether it handles - * throwables or not. + * The {@link PatternParser} checks this flag when processing the + * {@code alwaysWriteExceptions} option: if no converter in the pattern handles + * throwables, the parser automatically appends a converter to ensure exceptions are still written. * </p> * - * @return true if this PatternConverter handles throwables + * @return {@code true} if this converter consumes and renders a {@link Throwable}, + * {@code false} otherwise */ public boolean handlesThrowable() { return false; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MaxLengthConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MaxLengthConverter.java index 1a2805c23b..432cd4f61f 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MaxLengthConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MaxLengthConverter.java @@ -99,4 +99,11 @@ public final class MaxLengthConverter extends LogEventPatternConverter { } } } + + @Override + public boolean handlesThrowable() { + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/RegexReplacementConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/RegexReplacementConverter.java index 4a21a753bb..27bd86ada7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/RegexReplacementConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/RegexReplacementConverter.java @@ -92,4 +92,11 @@ public final class RegexReplacementConverter extends LogEventPatternConverter { } toAppendTo.append(pattern.matcher(buf.toString()).replaceAll(substitution)); } + + @Override + public boolean handlesThrowable() { + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/StyleConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/StyleConverter.java index 40267f6155..d6c0d123d8 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/StyleConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/StyleConverter.java @@ -129,12 +129,9 @@ public final class StyleConverter extends LogEventPatternConverter implements An @Override public boolean handlesThrowable() { - for (final PatternFormatter formatter : patternFormatters) { - if (formatter.handlesThrowable()) { - return true; - } - } - return false; + return patternFormatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); } /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/VariablesNotEmptyReplacementConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/VariablesNotEmptyReplacementConverter.java index e5d2ba9ecf..9929dc5a2d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/VariablesNotEmptyReplacementConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/VariablesNotEmptyReplacementConverter.java @@ -118,4 +118,11 @@ public final class VariablesNotEmptyReplacementConverter extends LogEventPattern } return true; } + + @Override + public boolean handlesThrowable() { + return formatters.stream() + .map(PatternFormatter::getConverter) + .anyMatch(LogEventPatternConverter::handlesThrowable); + } } diff --git a/src/changelog/.2.x.x/3920-nested-throwables.xml b/src/changelog/.2.x.x/3920-nested-throwables.xml new file mode 100644 index 0000000000..a8c9b13cfe --- /dev/null +++ b/src/changelog/.2.x.x/3920-nested-throwables.xml @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns="https://logging.apache.org/xml/ns"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation=" + https://logging.apache.org/xml/ns + https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"; + type="fixed"> + <issue id="3920" link="https://github.com/apache/logging-log4j2/pull/3920"/> + <description format="asciidoc"> + Fix detection of throwable converters inside nested patterns when applying `alwaysWriteExceptions`. + </description> +</entry>
