From 537686e4ef9fc51d5efcb9ff3c69e729949ff875 Mon Sep 17 00:00:00 2001 From: Valentin Zakharov Date: Wed, 20 May 2026 12:41:51 +0200 Subject: [PATCH 1/4] Fix DDSpan.addThrowable crashing when exception getMessage() throws --- .../main/java/datadog/trace/core/DDSpan.java | 4 +- .../datadog/trace/core/util/StackTraces.java | 45 +++++++++++++++++-- .../java/datadog/trace/core/DDSpanTest.java | 35 +++++++++++++++ .../trace/core/util/StackTracesTest.java | 21 +++++++++ .../trace/core/util/TestThrowables.java | 23 ++++++++++ 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/util/TestThrowables.java diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 2c62819e97a..310e6688c16 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/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 diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java index 3c6a00e599b..c47a4e99ab8 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java +++ b/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,45 @@ public final class StackTraces { private StackTraces() {} + /** + * Safely retrieves the message from a throwable. Returns {@code null} if {@code t} is {@code + * null}, or a diagnostic string of the form {@code "(Exception message unavailable for ClassName: + * getMessage() threw ExceptionType)"} if {@link Throwable#getMessage()} itself throws. + * 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. + */ + 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() + + ")"; + } + } + 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. + trace = + t.getClass().getName() + + System.lineSeparator() + + Arrays.stream(t.getStackTrace()) + .map(f -> "\tat " + f) + .collect(Collectors.joining(System.lineSeparator())); + } try { return truncate(trace, maxChars); } catch (Exception e) { @@ -43,6 +79,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() diff --git a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java b/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java index ec18c7b91e8..611c8806767 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java +++ b/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 ", diff --git a/dd-trace-core/src/test/java/datadog/trace/core/util/StackTracesTest.java b/dd-trace-core/src/test/java/datadog/trace/core/util/StackTracesTest.java index 05e14e33f52..ba4bc51d4f2 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/util/StackTracesTest.java +++ b/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) { diff --git a/dd-trace-core/src/test/java/datadog/trace/core/util/TestThrowables.java b/dd-trace-core/src/test/java/datadog/trace/core/util/TestThrowables.java new file mode 100644 index 00000000000..42885a4af45 --- /dev/null +++ b/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"); + } + }; + } +} From b87c75b28b056c9eca18074f3a965e4ba690af79 Mon Sep 17 00:00:00 2001 From: Valentin Zakharov Date: Thu, 21 May 2026 10:50:50 +0200 Subject: [PATCH 2/4] Add Javadoc to safeGetMessage and getStackTrace --- .../datadog/trace/core/util/StackTraces.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java index c47a4e99ab8..6d084f03293 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java @@ -15,12 +15,17 @@ public final class StackTraces { private StackTraces() {} /** - * Safely retrieves the message from a throwable. Returns {@code null} if {@code t} is {@code - * null}, or a diagnostic string of the form {@code "(Exception message unavailable for ClassName: - * getMessage() threw ExceptionType)"} if {@link Throwable#getMessage()} itself throws. - * Third-party exception classes occasionally use formatting utilities (e.g. {@code + * Safely retrieves the message from a throwable. + * + *

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) { @@ -37,6 +42,19 @@ public static String safeGetMessage(Throwable t) { } } + /** + * Returns the stack trace of {@code t} as a string, truncated to {@code maxChars} characters. + * + *

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) { String trace; try { From 52e75adc2b28048a6468a0e30f142087b4baf065 Mon Sep 17 00:00:00 2001 From: Valentin Zakharov Date: Thu, 21 May 2026 11:01:15 +0200 Subject: [PATCH 3/4] Protect getStackTrace() fallback against getStackTrace() also throwing --- .../datadog/trace/core/util/StackTraces.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java index 6d084f03293..d27d0e75197 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java @@ -64,12 +64,16 @@ public static String getStackTrace(Throwable t, int maxChars) { } catch (Exception ignored) { // printStackTrace() failed (e.g. getMessage() throws inside toString()). // Reconstruct from getStackTrace() so the call site is still locatable. - trace = - t.getClass().getName() - + System.lineSeparator() - + Arrays.stream(t.getStackTrace()) - .map(f -> "\tat " + f) - .collect(Collectors.joining(System.lineSeparator())); + 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); From 4dc5ed8869c4293710693dfb85fc7c6b803a5ccb Mon Sep 17 00:00:00 2001 From: Valentin Zakharov Date: Thu, 21 May 2026 12:45:48 +0200 Subject: [PATCH 4/4] Fix spotless formatting in StackTraces Javadoc --- .../src/main/java/datadog/trace/core/util/StackTraces.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java index d27d0e75197..c39d3fba63b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/util/StackTraces.java @@ -22,10 +22,9 @@ private StackTraces() {} * 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 + * @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) {