Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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<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
*/
Expand All @@ -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() //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ private EncodingPatternConverter(final List<PatternFormatter> 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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*
* <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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,11 @@ private static boolean sequenceRegionMatches(
}
return true;
}

@Override
public boolean handlesThrowable() {
return formatters.stream()
.map(PatternFormatter::getConverter)
.anyMatch(LogEventPatternConverter::handlesThrowable);
}
}
12 changes: 12 additions & 0 deletions src/changelog/.2.x.x/3920-nested-throwables.xml
Original file line number Diff line number Diff line change
@@ -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>
Loading