Fix DDSpan.addThrowable crashing when exception getMessage() throws (#11428)

Fix DDSpan.addThrowable crashing when exception getMessage() throws

Add Javadoc to safeGetMessage and getStackTrace

Protect getStackTrace() fallback against getStackTrace() also throwing

Fix spotless formatting in StackTraces Javadoc

Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

Author: ValentinZakharov

dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java

@@ -354,10 +354,10 @@ public DDSpan addThrowable(final Throwable error) {
   @Override
   public DDSpan addThrowable(Throwable error, byte errorPriority) {
     if (null != error) {
-      String message = error.getMessage();
+      String message = StackTraces.safeGetMessage(error);
       if (!"broken pipe".equalsIgnoreCase(message)
           && (error.getCause() == null
-              || !"broken pipe".equalsIgnoreCase(error.getCause().getMessage()))) {
+              || !"broken pipe".equalsIgnoreCase(StackTraces.safeGetMessage(error.getCause())))) {
         // broken pipes happen when clients abort connections,
         // which might happen because the application is overloaded
         // or warming up - capturing the stack trace and keeping

dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java

@@ -5,6 +5,7 @@
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -13,10 +14,66 @@
 public final class StackTraces {
   private StackTraces() {}
 
+  /**
+   * Safely retrieves the message from a throwable.
+   *
+   * <p>Third-party exception classes occasionally use formatting utilities (e.g. {@code
+   * java.text.MessageFormat}) inside {@code getMessage()}, which can throw when the pattern
+   * contains non-integer placeholders.
+   *
+   * @param t the throwable to retrieve the message from
+   * @return {@code null} if {@code t} is {@code null}; the result of {@link Throwable#getMessage()}
+   *     on success; or a diagnostic string of the form {@code "(Exception message unavailable for
+   *     ClassName: getMessage() threw ExceptionType)"} if {@code getMessage()} throws
+   */
+  public static String safeGetMessage(Throwable t) {
+    if (t == null) {
+      return null;
+    }
+    try {
+      return t.getMessage();
+    } catch (Exception e) {
+      return "(Exception message unavailable for "
+          + t.getClass().getSimpleName()
+          + ": getMessage() threw "
+          + e.getClass().getSimpleName()
+          + ")";
+    }
+  }
+
+  /**
+   * Returns the stack trace of {@code t} as a string, truncated to {@code maxChars} characters.
+   *
+   * <p>Uses {@link Throwable#printStackTrace(java.io.PrintWriter)} to produce the full trace
+   * including {@code Caused by} and {@code Suppressed} chains. If {@code printStackTrace} itself
+   * throws (e.g. because {@link Throwable#getMessage()} throws inside {@code toString()}), falls
+   * back to reconstructing the trace from {@link Throwable#getStackTrace()} so the call site
+   * remains locatable.
+   *
+   * @param t the throwable to format
+   * @param maxChars maximum length of the returned string
+   * @return the stack trace string, truncated if necessary
+   */
   public static String getStackTrace(Throwable t, int maxChars) {
-    StringWriter sw = new StringWriter();
-    t.printStackTrace(new PrintWriter(sw));
-    String trace = sw.toString();
+    String trace;
+    try {
+      StringWriter sw = new StringWriter();
+      t.printStackTrace(new PrintWriter(sw));
+      trace = sw.toString();
+    } catch (Exception ignored) {
+      // printStackTrace() failed (e.g. getMessage() throws inside toString()).
+      // Reconstruct from getStackTrace() so the call site is still locatable.
+      try {
+        trace =
+            t.getClass().getName()
+                + System.lineSeparator()
+                + Arrays.stream(t.getStackTrace())
+                    .map(f -> "\tat " + f)
+                    .collect(Collectors.joining(System.lineSeparator()));
+      } catch (Exception ignored2) {
+        trace = t.getClass().getName();
+      }
+    }
     try {
       return truncate(trace, maxChars);
     } catch (Exception e) {
@@ -43,6 +100,9 @@ static String truncate(String trace, int maxChars) {
     /* last-ditch centre cut to guarantee the limit */
     String cutMessage = "\t... trace centre-cut to " + maxChars + " chars ...";
     int retainedLength = maxChars - cutMessage.length() - 2; // 2 for the newlines
+    if (retainedLength <= 0) {
+      return cutMessage + System.lineSeparator();
+    }
     int half = retainedLength / 2;
     return trace.substring(0, half)
         + System.lineSeparator()

dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java

@@ -10,6 +10,7 @@
 import static datadog.trace.core.DDSpanContext.SPAN_SAMPLING_RULE_RATE_TAG;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
@@ -33,6 +34,7 @@
 import datadog.trace.common.writer.ListWriter;
 import datadog.trace.core.propagation.ExtractedContext;
 import datadog.trace.core.propagation.PropagationTags;
+import datadog.trace.core.util.TestThrowables;
 import java.io.Closeable;
 import java.io.IOException;
 import java.lang.reflect.Field;
@@ -447,6 +449,39 @@ void nullExceptionSafeToAdd() {
     assertNull(span.getTag(DDTags.ERROR_STACK));
   }
 
+  @Test
+  void addThrowableDoesNotFailWhenGetMessageThrows() {
+    DDSpan span = (DDSpan) tracer.buildSpan("datadog", "root").start();
+
+    span.addThrowable(TestThrowables.throwingGetMessage());
+
+    assertTrue(span.isError());
+    assertNotNull(span.getTag(DDTags.ERROR_TYPE));
+    assertNotNull(
+        span.getTag(DDTags.ERROR_STACK),
+        "stack trace must be captured even when getMessage() throws");
+    assertTrue(((String) span.getTag(DDTags.ERROR_MSG)).contains("Exception message unavailable"));
+  }
+
+  @Test
+  void addThrowableDoesNotFailWhenGetCauseMessageThrows() {
+    // Outer exception has a normal message, so the broken-pipe check reaches
+    // getCause().getMessage()
+    RuntimeException error =
+        new RuntimeException("outer message", TestThrowables.throwingGetMessage());
+    DDSpan span = (DDSpan) tracer.buildSpan("datadog", "root").start();
+
+    span.addThrowable(error);
+
+    assertTrue(span.isError());
+    assertNotNull(span.getTag(DDTags.ERROR_TYPE));
+    assertNotNull(
+        span.getTag(DDTags.ERROR_STACK),
+        "stack trace must be captured even when cause's getMessage() throws");
+    // Outer getMessage() works normally — message must be recorded
+    assertEquals("outer message", span.getTag(DDTags.ERROR_MSG));
+  }
+
   @TableTest({
     "scenario         | rate | limit     ",
     "rate=1.0 lim=10  | 1.0  | 10        ",

dd-trace-core/src/test/java/datadog/trace/core/util/StackTracesTest.java

@@ -1,9 +1,12 @@
 package datadog.trace.core.util;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -64,6 +67,24 @@ class StackTracesTest {
           + "    at com.example.util.ResourceManager.close(ResourceManager.java:21)\n"
           + "    ... 3 more\n";
 
+  // --- safeGetMessage ---
+
+  @Test
+  void safeGetMessageReturnsFallbackWhenGetMessageThrows() {
+    String message = StackTraces.safeGetMessage(TestThrowables.throwingGetMessage());
+    assertTrue(
+        message.contains("Exception message unavailable"), "must indicate message is unavailable");
+    assertTrue(
+        message.contains("IllegalArgumentException"), "must include secondary exception type");
+  }
+
+  @Test
+  void safeGetMessageReturnsNullForNullInput() {
+    assertNull(StackTraces.safeGetMessage(null));
+  }
+
+  // --- getStackTrace with broken getMessage ---
+
   @ParameterizedTest(name = "truncation limit {0}")
   @MethodSource("testTruncateArguments")
   void testTruncate(int limit, String expected) {

dd-trace-core/src/test/java/datadog/trace/core/util/TestThrowables.java

@@ -0,0 +1,23 @@
+package datadog.trace.core.util;
+
+import java.text.MessageFormat;
+
+/** Test helpers for throwables with non-standard {@code getMessage()} behaviour. */
+public final class TestThrowables {
+  private TestThrowables() {}
+
+  /**
+   * Returns a {@link RuntimeException} whose {@link Throwable#getMessage()} throws {@link
+   * IllegalArgumentException} via {@link MessageFormat} with non-integer placeholders — simulating
+   * the third-party exception that triggered the production bug in {@code DDSpan.addThrowable}.
+   */
+  public static RuntimeException throwingGetMessage() {
+    return new RuntimeException() {
+      @Override
+      public String getMessage() {
+        return MessageFormat.format(
+            "Timeout after {TotalMilliseconds}ms matching pattern {Pattern}", "arg0", "arg1");
+      }
+    };
+  }
+}