From 5c8542bcded945401a86269af0f00c7a275a94fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 May 2025 22:53:52 +0000 Subject: [PATCH 1/4] Initial plan for issue From 1c5a2b7cdce02a5f809c7e0f20edf21ade695ee6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 May 2025 22:59:12 +0000 Subject: [PATCH 2/4] Enhance exception serialization with improved stack trace handling Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com> --- .../microsoft/durabletask/FailureDetails.java | 58 +++++++++++-- .../ExceptionSerializationTest.java | 83 +++++++++++++++++++ 2 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 client/src/test/java/com/microsoft/durabletask/ExceptionSerializationTest.java diff --git a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java index ad3f1ba1..e195772c 100644 --- a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java +++ b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java @@ -35,7 +35,19 @@ public final class FailureDetails { } FailureDetails(Exception exception) { - this(exception.getClass().getName(), exception.getMessage(), getFullStackTrace(exception), false); + // Use the most specific exception in the chain for error type + String errorType = exception.getClass().getName(); + String errorMessage = exception.getMessage(); + + // Preserve null messages as empty string to match existing behavior + if (errorMessage == null) { + errorMessage = ""; + } + + this.errorType = errorType; + this.errorMessage = errorMessage; + this.stackTrace = getFullStackTrace(exception); + this.isNonRetriable = false; } FailureDetails(TaskFailureDetails proto) { @@ -113,14 +125,48 @@ public boolean isCausedBy(Class exceptionClass) { } static String getFullStackTrace(Throwable e) { - StackTraceElement[] elements = e.getStackTrace(); + StringBuilder sb = new StringBuilder(); + + // Process the exception chain recursively + appendExceptionDetails(sb, e, null); + + return sb.toString(); + } + + private static void appendExceptionDetails(StringBuilder sb, Throwable ex, StackTraceElement[] parentStackTrace) { + if (ex == null) { + return; + } - // Plan for 256 characters per stack frame (which is likely on the high-end) - StringBuilder sb = new StringBuilder(elements.length * 256); - for (StackTraceElement element : elements) { + // Add the exception class name and message + sb.append(ex.getClass().getName()); + String message = ex.getMessage(); + if (message != null) { + sb.append(": ").append(message); + } + sb.append(System.lineSeparator()); + + // Add the stack trace elements + StackTraceElement[] currentStackTrace = ex.getStackTrace(); + for (StackTraceElement element : currentStackTrace) { sb.append("\tat ").append(element.toString()).append(System.lineSeparator()); } - return sb.toString(); + + // Handle any suppressed exceptions + Throwable[] suppressed = ex.getSuppressed(); + if (suppressed != null && suppressed.length > 0) { + for (Throwable s : suppressed) { + sb.append("\tSuppressed: "); + appendExceptionDetails(sb, s, currentStackTrace); + } + } + + // Handle cause (inner exception) + Throwable cause = ex.getCause(); + if (cause != null && cause != ex) { // Avoid infinite recursion + sb.append("Caused by: "); + appendExceptionDetails(sb, cause, currentStackTrace); + } } TaskFailureDetails toProto() { diff --git a/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationTest.java b/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationTest.java new file mode 100644 index 00000000..6ef8fdf1 --- /dev/null +++ b/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationTest.java @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.durabletask; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for exception serialization and deserialization in FailureDetails. + */ +public class ExceptionSerializationTest { + + @Test + public void testSimpleException() { + String message = "Simple exception message"; + RuntimeException exception = new RuntimeException(message); + FailureDetails details = new FailureDetails(exception); + + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals(message, details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + assertTrue(details.getStackTrace().contains("at com.microsoft.durabletask.ExceptionSerializationTest")); + } + + @Test + public void testMultilineMessage() { + String multilineMessage = "Line 1\nLine 2\nLine 3"; + RuntimeException exception = new RuntimeException(multilineMessage); + FailureDetails details = new FailureDetails(exception); + + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals(multilineMessage, details.getErrorMessage()); + assertTrue(details.getStackTrace().contains("at com.microsoft.durabletask.ExceptionSerializationTest")); + } + + @Test + public void testNestedException() { + String innerMessage = "Inner exception"; + String outerMessage = "Outer exception"; + + Exception innerException = new IllegalArgumentException(innerMessage); + RuntimeException outerException = new RuntimeException(outerMessage, innerException); + + FailureDetails details = new FailureDetails(outerException); + + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals(outerMessage, details.getErrorMessage()); + + String stackTrace = details.getStackTrace(); + assertNotNull(stackTrace); + + // The stack trace should contain information about both exceptions + assertTrue(stackTrace.contains(outerMessage), "Stack trace should contain outer exception message"); + assertTrue(stackTrace.contains(innerMessage), "Stack trace should contain inner exception message"); + assertTrue(stackTrace.contains("Caused by: java.lang.IllegalArgumentException"), + "Stack trace should include 'Caused by' section"); + } + + @Test + public void testNonStandardExceptionToString() { + CustomException exception = new CustomException("Custom message"); + FailureDetails details = new FailureDetails(exception); + + assertEquals(CustomException.class.getName(), details.getErrorType()); + assertEquals("Custom message", details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + } + + /** + * Custom exception class with a non-standard toString implementation. + */ + private static class CustomException extends Exception { + public CustomException(String message) { + super(message); + } + + @Override + public String toString() { + return "CUSTOM_EXCEPTION_FORMAT: " + getMessage(); + } + } +} \ No newline at end of file From 7228f17df272c5bce15ec4d0ce87f97c965bbb27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 May 2025 23:00:55 +0000 Subject: [PATCH 3/4] Add integration tests for exception serialization and improve stack trace formatting Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com> --- .../microsoft/durabletask/FailureDetails.java | 30 ++++- ...ExceptionSerializationIntegrationTest.java | 122 ++++++++++++++++++ 2 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 client/src/test/java/com/microsoft/durabletask/ExceptionSerializationIntegrationTest.java diff --git a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java index e195772c..cb4c7931 100644 --- a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java +++ b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java @@ -148,8 +148,18 @@ private static void appendExceptionDetails(StringBuilder sb, Throwable ex, Stack // Add the stack trace elements StackTraceElement[] currentStackTrace = ex.getStackTrace(); - for (StackTraceElement element : currentStackTrace) { - sb.append("\tat ").append(element.toString()).append(System.lineSeparator()); + int framesInCommon = 0; + if (parentStackTrace != null) { + framesInCommon = countCommonFrames(currentStackTrace, parentStackTrace); + } + + int framesToPrint = currentStackTrace.length - framesInCommon; + for (int i = 0; i < framesToPrint; i++) { + sb.append("\tat ").append(currentStackTrace[i].toString()).append(System.lineSeparator()); + } + + if (framesInCommon > 0) { + sb.append("\t... ").append(framesInCommon).append(" more").append(System.lineSeparator()); } // Handle any suppressed exceptions @@ -168,6 +178,22 @@ private static void appendExceptionDetails(StringBuilder sb, Throwable ex, Stack appendExceptionDetails(sb, cause, currentStackTrace); } } + + /** + * Count frames in common between two stack traces, starting from the end. + * This helps produce more concise stack traces for chained exceptions. + */ + private static int countCommonFrames(StackTraceElement[] trace1, StackTraceElement[] trace2) { + int m = trace1.length - 1; + int n = trace2.length - 1; + int count = 0; + while (m >= 0 && n >= 0 && trace1[m].equals(trace2[n])) { + m--; + n--; + count++; + } + return count; + } TaskFailureDetails toProto() { return TaskFailureDetails.newBuilder() diff --git a/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationIntegrationTest.java b/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationIntegrationTest.java new file mode 100644 index 00000000..e57c4581 --- /dev/null +++ b/client/src/test/java/com/microsoft/durabletask/ExceptionSerializationIntegrationTest.java @@ -0,0 +1,122 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.durabletask; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.util.concurrent.TimeoutException; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Integration tests for validating the serialization of exceptions in various scenarios. + */ +@Tag("integration") +@ExtendWith(TestRetryExtension.class) +public class ExceptionSerializationIntegrationTest extends IntegrationTestBase { + + @RetryingTest + void testMultilineExceptionMessage() throws TimeoutException { + final String orchestratorName = "MultilineExceptionOrchestrator"; + final String multilineErrorMessage = "Line 1\nLine 2\nLine 3"; + + DurableTaskGrpcWorker worker = this.createWorkerBuilder() + .addOrchestrator(orchestratorName, ctx -> { + throw new RuntimeException(multilineErrorMessage); + }) + .buildAndStart(); + + DurableTaskClient client = new DurableTaskGrpcClientBuilder().build(); + try (worker; client) { + String instanceId = client.scheduleNewOrchestrationInstance(orchestratorName, 0); + OrchestrationMetadata instance = client.waitForInstanceCompletion(instanceId, defaultTimeout, true); + assertNotNull(instance); + assertEquals(OrchestrationRuntimeStatus.FAILED, instance.getRuntimeStatus()); + + FailureDetails details = instance.getFailureDetails(); + assertNotNull(details); + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals(multilineErrorMessage, details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + } + } + + @RetryingTest + void testNestedExceptions() throws TimeoutException { + final String orchestratorName = "NestedExceptionOrchestrator"; + final String innerMessage = "Inner exception"; + final String outerMessage = "Outer exception"; + + DurableTaskGrpcWorker worker = this.createWorkerBuilder() + .addOrchestrator(orchestratorName, ctx -> { + Exception innerException = new IllegalArgumentException(innerMessage); + throw new RuntimeException(outerMessage, innerException); + }) + .buildAndStart(); + + DurableTaskClient client = new DurableTaskGrpcClientBuilder().build(); + try (worker; client) { + String instanceId = client.scheduleNewOrchestrationInstance(orchestratorName, 0); + OrchestrationMetadata instance = client.waitForInstanceCompletion(instanceId, defaultTimeout, true); + assertNotNull(instance); + assertEquals(OrchestrationRuntimeStatus.FAILED, instance.getRuntimeStatus()); + + FailureDetails details = instance.getFailureDetails(); + assertNotNull(details); + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals(outerMessage, details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + + // Verify both exceptions are in the stack trace + String stackTrace = details.getStackTrace(); + assertTrue(stackTrace.contains(outerMessage), "Stack trace should contain outer exception message"); + assertTrue(stackTrace.contains(innerMessage), "Stack trace should contain inner exception message"); + assertTrue(stackTrace.contains("Caused by: java.lang.IllegalArgumentException"), + "Stack trace should include 'Caused by' section for inner exception"); + } + } + + @RetryingTest + void testCustomExceptionWithNonStandardToString() throws TimeoutException { + final String orchestratorName = "CustomExceptionOrchestrator"; + final String customMessage = "Custom exception message"; + + DurableTaskGrpcWorker worker = this.createWorkerBuilder() + .addOrchestrator(orchestratorName, ctx -> { + throw new CustomException(customMessage); + }) + .buildAndStart(); + + DurableTaskClient client = new DurableTaskGrpcClientBuilder().build(); + try (worker; client) { + String instanceId = client.scheduleNewOrchestrationInstance(orchestratorName, 0); + OrchestrationMetadata instance = client.waitForInstanceCompletion(instanceId, defaultTimeout, true); + assertNotNull(instance); + assertEquals(OrchestrationRuntimeStatus.FAILED, instance.getRuntimeStatus()); + + FailureDetails details = instance.getFailureDetails(); + assertNotNull(details); + String expectedType = CustomException.class.getName(); + assertEquals(expectedType, details.getErrorType()); + assertEquals(customMessage, details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + } + } + + /** + * Custom exception class with a non-standard toString implementation. + */ + private static class CustomException extends RuntimeException { + public CustomException(String message) { + super(message); + } + + @Override + public String toString() { + return "CUSTOM_EXCEPTION_FORMAT: " + getMessage(); + } + } +} \ No newline at end of file From 88f289e86daec4759b03914d543fa2b32abf3987 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 May 2025 23:03:45 +0000 Subject: [PATCH 4/4] Finalize exception serialization implementation with comprehensive tests Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com> --- .../microsoft/durabletask/FailureDetails.java | 39 +++++- .../durabletask/ComplexExceptionTest.java | 112 ++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 client/src/test/java/com/microsoft/durabletask/ComplexExceptionTest.java diff --git a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java index cb4c7931..7f91bbf6 100644 --- a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java +++ b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java @@ -34,6 +34,16 @@ public final class FailureDetails { this.isNonRetriable = isNonRetriable; } + /** + * Creates a new failure details object from an exception. + * This constructor captures comprehensive exception information including: + * - Exception type + * - Exception message (including multiline messages) + * - Complete stack trace with inner/nested exceptions + * - Suppressed exceptions + * + * @param exception The exception to create failure details from + */ FailureDetails(Exception exception) { // Use the most specific exception in the chain for error type String errorType = exception.getClass().getName(); @@ -124,7 +134,18 @@ public boolean isCausedBy(Class exceptionClass) { } } + /** + * Generates a comprehensive stack trace string from a throwable, including inner exceptions, suppressed exceptions, + * and handling circular references. The format closely resembles Java's standard exception printing format. + * + * @param e The throwable to convert to a stack trace string + * @return A formatted stack trace string + */ static String getFullStackTrace(Throwable e) { + if (e == null) { + return ""; + } + StringBuilder sb = new StringBuilder(); // Process the exception chain recursively @@ -166,16 +187,24 @@ private static void appendExceptionDetails(StringBuilder sb, Throwable ex, Stack Throwable[] suppressed = ex.getSuppressed(); if (suppressed != null && suppressed.length > 0) { for (Throwable s : suppressed) { - sb.append("\tSuppressed: "); - appendExceptionDetails(sb, s, currentStackTrace); + if (s != ex) { // Avoid circular references + sb.append("\tSuppressed: "); + appendExceptionDetails(sb, s, currentStackTrace); + } else { + sb.append("\tSuppressed: [CIRCULAR REFERENCE]").append(System.lineSeparator()); + } } } // Handle cause (inner exception) Throwable cause = ex.getCause(); - if (cause != null && cause != ex) { // Avoid infinite recursion - sb.append("Caused by: "); - appendExceptionDetails(sb, cause, currentStackTrace); + if (cause != null) { + if (cause != ex) { // Avoid direct circular references + sb.append("Caused by: "); + appendExceptionDetails(sb, cause, currentStackTrace); + } else { + sb.append("Caused by: [CIRCULAR REFERENCE]").append(System.lineSeparator()); + } } } diff --git a/client/src/test/java/com/microsoft/durabletask/ComplexExceptionTest.java b/client/src/test/java/com/microsoft/durabletask/ComplexExceptionTest.java new file mode 100644 index 00000000..aa761e66 --- /dev/null +++ b/client/src/test/java/com/microsoft/durabletask/ComplexExceptionTest.java @@ -0,0 +1,112 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.durabletask; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for handling complex exception serialization scenarios. + */ +public class ComplexExceptionTest { + + @Test + public void testDeepNestedExceptions() { + // Create a chain of 5 nested exceptions + Exception level5 = new IllegalArgumentException("Level 5 exception"); + Exception level4 = new IllegalStateException("Level 4 exception", level5); + Exception level3 = new RuntimeException("Level 3 exception", level4); + Exception level2 = new Exception("Level 2 exception", level3); + Exception level1 = new Exception("Level 1 exception", level2); + + FailureDetails details = new FailureDetails(level1); + + assertEquals("java.lang.Exception", details.getErrorType()); + assertEquals("Level 1 exception", details.getErrorMessage()); + + String stackTrace = details.getStackTrace(); + assertNotNull(stackTrace); + + // Verify all exception levels are present in the stack trace + assertTrue(stackTrace.contains("Level 1 exception")); + assertTrue(stackTrace.contains("Caused by: java.lang.Exception: Level 2 exception")); + assertTrue(stackTrace.contains("Caused by: java.lang.RuntimeException: Level 3 exception")); + assertTrue(stackTrace.contains("Caused by: java.lang.IllegalStateException: Level 4 exception")); + assertTrue(stackTrace.contains("Caused by: java.lang.IllegalArgumentException: Level 5 exception")); + } + + @Test + public void testExceptionWithSuppressedExceptions() { + Exception mainException = new RuntimeException("Main exception"); + Exception suppressed1 = new IllegalArgumentException("Suppressed exception 1"); + Exception suppressed2 = new IllegalStateException("Suppressed exception 2"); + + mainException.addSuppressed(suppressed1); + mainException.addSuppressed(suppressed2); + + FailureDetails details = new FailureDetails(mainException); + + assertEquals("java.lang.RuntimeException", details.getErrorType()); + assertEquals("Main exception", details.getErrorMessage()); + + String stackTrace = details.getStackTrace(); + assertNotNull(stackTrace); + + // Verify suppressed exceptions are in the stack trace + assertTrue(stackTrace.contains("Main exception")); + assertTrue(stackTrace.contains("Suppressed: java.lang.IllegalArgumentException: Suppressed exception 1")); + assertTrue(stackTrace.contains("Suppressed: java.lang.IllegalStateException: Suppressed exception 2")); + } + + @Test + public void testNullMessageException() { + NullPointerException exception = new NullPointerException(); // NPE typically has null message + + FailureDetails details = new FailureDetails(exception); + + assertEquals("java.lang.NullPointerException", details.getErrorType()); + assertEquals("", details.getErrorMessage()); // Should convert null to empty string + assertNotNull(details.getStackTrace()); + } + + @Test + public void testCircularExceptionReference() { + try { + // Create an exception with a circular reference (should be handled gracefully) + ExceptionWithCircularReference ex = new ExceptionWithCircularReference("Circular"); + ex.setCircularCause(); + + FailureDetails details = new FailureDetails(ex); + + assertEquals(ExceptionWithCircularReference.class.getName(), details.getErrorType()); + assertEquals("Circular", details.getErrorMessage()); + assertNotNull(details.getStackTrace()); + + // No infinite loop, test passes if we get here + } catch (StackOverflowError e) { + fail("StackOverflowError occurred with circular exception reference"); + } + } + + /** + * Exception class that can create a circular reference in the cause chain. + */ + private static class ExceptionWithCircularReference extends Exception { + public ExceptionWithCircularReference(String message) { + super(message); + } + + public void setCircularCause() { + try { + // Use reflection to set the cause field directly to this exception + // to create a circular reference + java.lang.reflect.Field causeField = Throwable.class.getDeclaredField("cause"); + causeField.setAccessible(true); + causeField.set(this, this); + } catch (Exception e) { + // Ignore any reflection errors, this is just for testing + } + } + } +} \ No newline at end of file