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

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

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() 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"); + } + }; + } +}