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 7a82831b960..33a3f2b6caa 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 @@ -44,6 +44,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; class PatternParserTest { @@ -93,6 +94,50 @@ void defaultPattern() { 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 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 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 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 */ @@ -103,7 +148,7 @@ void testCustomPattern() { final StringMap mdc = ContextDataFactory.createContextData(); mdc.putValue("loginId", "Fred"); // The line number of the Throwable definition - final int nextLineNumber = 107; + final int nextLineNumber = 152; // Adjust this when the code above changes! final Throwable t = new Throwable(); final StackTraceElement[] elements = t.getStackTrace(); final Log4jLogEvent event = Log4jLogEvent.newBuilder() // 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 b7b5e350180..98d69acadec 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 void format(final LogEvent event, final StringBuilder toAppendTo) { 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 970517f69ff..2458e2cb708 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 @@ private EncodingPatternConverter(final List formatters, final @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 904d4981698..e71edb61476 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 @@ void parseSubstitution(final LogEvent event, final StringBuilder substitutionBuf 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 29663b4f5a8..6f72d2f47ea 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 @@ String getLevelStyle(final Level level) { @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 4d4bc2cbc75..4c175abdb49 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 void format(final Object obj, final StringBuilder output) { } /** - * Normally pattern converters are not meant to handle Exceptions although few pattern converters might. + * Tests whether this pattern converter is renders a {@link Throwable}. + * *

- * 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. *

* - * @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 1a2805c23b6..432cd4f61fd 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 void format(final LogEvent event, final StringBuilder toAppendTo) { } } } + + @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 4a21a753bb0..27bd86ada7f 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 void format(final LogEvent event, final StringBuilder toAppendTo) { } 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 40267f6155c..d6c0d123d8f 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 void format(final LogEvent event, final StringBuilder toAppendTo) { @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 7fda5791d5e..ca7cdf71e52 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 @@ -119,4 +119,11 @@ private static boolean sequenceRegionMatches( } 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 00000000000..a8c9b13cfe3 --- /dev/null +++ b/src/changelog/.2.x.x/3920-nested-throwables.xml @@ -0,0 +1,12 @@ + + + + + Fix detection of throwable converters inside nested patterns when applying `alwaysWriteExceptions`. + +