Take Throwable#toString() into account while rendering stack traces in Pattern Layout (#4033)

Signed-off-by: Jongmin Chung <chungjm0711@gmail.com>
Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>

Author: 2 people

log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java

@@ -59,11 +59,8 @@ void parameters_should_be_garbage_collected(final TestInfo testInfo) throws Thro
                 assertThat(messages).hasSize(4);
                 assertThat(messages.get(0)).isEqualTo("Message with parameter %s", parameter.value);
                 assertThat(messages.get(1)).isEqualTo(parameter.value);
-                assertThat(messages.get(2))
-                        .startsWith(String.format("test%n%s: %s", ObjectThrowable.class.getName(), parameter.value));
-                assertThat(messages.get(3))
-                        .startsWith(
-                                String.format("test hello%n%s: %s", ObjectThrowable.class.getName(), parameter.value));
+                assertThat(messages.get(2)).startsWith(String.format("test%n%s", new ObjectThrowable(parameter)));
+                assertThat(messages.get(3)).startsWith(String.format("test hello%n%s", new ObjectThrowable(parameter)));
 
                 // Return the GC subject
                 return parameter;

log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverterTest.java

@@ -69,6 +69,23 @@ public String toString() {
         }
     }
 
+    /**
+     * Test exception whose {@link #toString()} is intentionally overridden to return a fixed value.
+     */
+    private static final class ToStringOverridingException extends RuntimeException {
+
+        private static final ToStringOverridingException INSTANCE = new ToStringOverridingException();
+
+        private ToStringOverridingException() {
+            super(EXCEPTION);
+        }
+
+        @Override
+        public String toString() {
+            return "foo";
+        }
+    }
+
     static Stream<SeparatorTestCase> separatorTestCases() {
         final String level = LEVEL.toString();
         return Stream.of(
@@ -204,6 +221,14 @@ void full_output_should_match_Throwable_printStackTrace(final String pattern) {
             assertThat(actualStackTrace).as("pattern=`%s`", effectivePattern).isEqualTo(expectedStackTrace);
         }
 
+        @Test
+        void full_output_should_use_custom_toString() {
+            final Throwable exception = ToStringOverridingException.INSTANCE;
+            final String expectedStackTrace = renderStackTraceUsingJava(exception);
+            final String actualStackTrace = convert(patternPrefix, exception);
+            assertThat(actualStackTrace).isEqualTo(expectedStackTrace);
+        }
+
         // This test does not provide `separator` and `suffix` options, since the reference output will be obtained from
         // `Throwable#printStackTrace()`, which doesn't take these into account.
         @ParameterizedTest
@@ -242,10 +267,14 @@ private String limitLines(final String text, final int maxLineCount) {
         }
 
         private String renderStackTraceUsingJava() {
+            return renderStackTraceUsingJava(EXCEPTION);
+        }
+
+        private String renderStackTraceUsingJava(final Throwable throwable) {
             final Charset charset = StandardCharsets.UTF_8;
             try (final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
                     final PrintStream printStream = new PrintStream(outputStream, false, charset.name())) {
-                EXCEPTION.printStackTrace(printStream);
+                throwable.printStackTrace(printStream);
                 printStream.flush();
                 return new String(outputStream.toByteArray(), charset);
             } catch (final Exception error) {
@@ -438,9 +467,13 @@ private static String normalizeStackTrace(final String stackTrace, final String
     }
 
     static String convert(final String pattern) {
+        return convert(pattern, EXCEPTION);
+    }
+
+    private static String convert(final String pattern, final Throwable throwable) {
         final List<PatternFormatter> patternFormatters = PATTERN_PARSER.parse(pattern, false, true, true);
         final LogEvent logEvent =
-                Log4jLogEvent.newBuilder().setThrown(EXCEPTION).setLevel(LEVEL).build();
+                Log4jLogEvent.newBuilder().setThrown(throwable).setLevel(LEVEL).build();
         final StringBuilder buffer = new StringBuilder();
         for (final PatternFormatter patternFormatter : patternFormatters) {
             patternFormatter.format(logEvent, buffer);

log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRenderer.java

@@ -138,12 +138,7 @@ private void renderCause(
     }
 
     static void renderThrowableMessage(final StringBuilder buffer, final Throwable throwable) {
-        final String message = throwable.getLocalizedMessage();
-        buffer.append(throwable.getClass().getName());
-        if (message != null) {
-            buffer.append(": ");
-            buffer.append(message);
-        }
+        buffer.append(throwable);
     }
 
     final void renderStackTraceElements(

src/changelog/.2.x.x/4033_fix_custom_throwable_to_sting.xml

@@ -0,0 +1,13 @@
+<?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="changed">
+    <issue id="3623" link="https://github.com/apache/logging-log4j2/issues/3623"/>
+    <issue id="4033" link="https://github.com/apache/logging-log4j2/pull/4033"/>
+    <description format="asciidoc">
+        Take `Throwable#toString()` into account while rendering stack traces in Pattern Layout.
+    </description>
+</entry>