From 068bf52e708bcf511399fd321b5a506aed3662c0 Mon Sep 17 00:00:00 2001 From: "naiyuantian@microsoft.com" Date: Thu, 16 Jul 2026 20:36:19 -0700 Subject: [PATCH 1/6] initial commit --- azurefunctions/build.gradle | 1 + .../middleware/ActivityMiddleware.java | 331 ++++++++++++++++++ ...nctions.internal.spi.middleware.Middleware | 3 +- .../middleware/ActivityMiddlewareTest.java | 286 +++++++++++++++ .../TestExceptionPropertiesProvider.java | 23 ++ .../PROTO_SOURCE_COMMIT_HASH | 2 +- .../protos/orchestrator_service.proto | 2 +- 7 files changed, 645 insertions(+), 3 deletions(-) create mode 100644 azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java create mode 100644 azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java create mode 100644 azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/TestExceptionPropertiesProvider.java diff --git a/azurefunctions/build.gradle b/azurefunctions/build.gradle index 4d991038..5563b5bd 100644 --- a/azurefunctions/build.gradle +++ b/azurefunctions/build.gradle @@ -40,6 +40,7 @@ dependencies { compileOnly "com.microsoft.azure.functions:azure-functions-java-spi:1.1.0" // Test dependencies + testImplementation "com.microsoft.azure.functions:azure-functions-java-spi:1.1.0" testImplementation 'org.mockito:mockito-core:5.21.0' testImplementation 'org.mockito:mockito-junit-jupiter:5.21.0' testImplementation platform('org.junit:junit-bom:5.14.2') diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java new file mode 100644 index 00000000..5d6721cc --- /dev/null +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -0,0 +1,331 @@ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for + * license information. + */ + +package com.microsoft.durabletask.azurefunctions.internal.middleware; + +import com.microsoft.azure.functions.internal.spi.middleware.Middleware; +import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareChain; +import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext; +import com.microsoft.durabletask.ExceptionPropertiesProvider; + +import java.lang.reflect.InvocationTargetException; +import java.util.Iterator; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.function.Supplier; +import java.util.logging.Logger; + +/** + * Durable Function Activity Middleware. + * + *

When an activity function throws, this middleware gives a registered + * {@link ExceptionPropertiesProvider} the chance to attach custom properties to the failure. If the + * provider returns any properties, the exception is reshaped into a serialized + * {@code TaskFailureDetails} JSON payload (matching the protobuf JSON shape) so the Durable Task + * host extension can surface the structured properties on {@code FailureDetails.Properties}. This + * mirrors the {@code durable-functions} JavaScript SDK's activity handler wrapper. + * + *

If no provider is registered, or it yields no properties for the thrown exception, the original + * exception is re-thrown untouched so the legacy failure behavior is preserved. + * + *

The provider is discovered via {@link ServiceLoader} (SPI): an application registers its + * implementation in {@code META-INF/services/com.microsoft.durabletask.ExceptionPropertiesProvider}. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +public class ActivityMiddleware implements Middleware { + + private static final String ACTIVITY_TRIGGER = "DurableActivityTrigger"; + private static final int MAX_INNER_FAILURE_DEPTH = 10; + private static final Logger LOGGER = Logger.getLogger(ActivityMiddleware.class.getName()); + + private static final Object PROVIDER_LOCK = new Object(); + private static volatile boolean providerLoaded = false; + private static ExceptionPropertiesProvider cachedProvider; + + // Visible for testing only. When non-null, this supplier replaces SPI discovery so unit tests + // can exercise the reshaping and pass-through behavior without registering a real provider. + private static Supplier providerSupplierOverride; + + @Override + public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exception { + String parameterName = context.getParameterName(ACTIVITY_TRIGGER); + if (parameterName == null) { + chain.doNext(context); + return; + } + + try { + chain.doNext(context); + } catch (Exception e) { + ExceptionPropertiesProvider provider = getProvider(); + if (provider == null) { + throw e; + } + + Throwable userException = unwrap(e); + Map properties = safeGetProperties(provider, userException); + if (properties == null || properties.isEmpty()) { + // No custom properties for this failure - preserve the original behavior. + throw e; + } + + throw new StructuredActivityFailure(buildFailureDetailsJson(userException, provider)); + } + } + + private static ExceptionPropertiesProvider getProvider() { + if (!providerLoaded) { + synchronized (PROVIDER_LOCK) { + if (!providerLoaded) { + cachedProvider = providerSupplierOverride != null + ? providerSupplierOverride.get() + : discoverProvider(); + providerLoaded = true; + } + } + } + return cachedProvider; + } + + private static ExceptionPropertiesProvider discoverProvider() { + // The provider is registered via SPI in the function app's jar. Depending on how the + // Azure Functions Java worker dispatches invocations, the thread context class loader may + // be the worker's class loader (which cannot see the app's META-INF/services registration) + // rather than the app class loader. Try several candidate class loaders and use the first + // one that yields a provider. The class loader that loaded this middleware is bundled with + // the app (durabletask-azure-functions is an app dependency), so it can see the app's SPI + // registration and is the most reliable fallback. + return discoverProvider(new ClassLoader[] { + Thread.currentThread().getContextClassLoader(), + ActivityMiddleware.class.getClassLoader(), + ExceptionPropertiesProvider.class.getClassLoader(), + }); + } + + // Visible for testing. Iterates the candidate class loaders in order and returns the first + // provider discovered via SPI, skipping nulls and duplicates. This is the seam that guards + // against the worker-thread class loader regression: discovery must not stop at the (possibly + // provider-blind) thread context class loader. + static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candidates) { + ClassLoader previous = null; + for (ClassLoader classLoader : candidates) { + if (classLoader == null || classLoader == previous) { + continue; + } + previous = classLoader; + try { + ServiceLoader loader = + ServiceLoader.load(ExceptionPropertiesProvider.class, classLoader); + Iterator iterator = loader.iterator(); + if (iterator.hasNext()) { + return iterator.next(); + } + } catch (Throwable t) { + // Discovery failures must not break activity execution; the feature is opt-in. + LOGGER.warning("Failed to load ExceptionPropertiesProvider via ServiceLoader using " + + classLoader + ": " + t); + } + } + return null; + } + + // Visible for testing. Overrides SPI discovery with the given supplier (may be {@code null} to + // simulate "no provider registered") and clears the cached provider so the next lookup re-runs. + static void setProviderSupplierForTesting(Supplier supplier) { + synchronized (PROVIDER_LOCK) { + providerSupplierOverride = supplier; + providerLoaded = false; + cachedProvider = null; + } + } + + // Visible for testing. Restores real SPI discovery and clears any cached provider so tests do + // not leak state into one another (the provider is cached in a static field). + static void resetProviderCacheForTesting() { + synchronized (PROVIDER_LOCK) { + providerSupplierOverride = null; + providerLoaded = false; + cachedProvider = null; + } + } + + private static Throwable unwrap(Throwable e) { + Throwable current = e; + while (current instanceof InvocationTargetException && current.getCause() != null) { + current = current.getCause(); + } + return current; + } + + private static Map safeGetProperties( + ExceptionPropertiesProvider provider, + Throwable exception) { + if (!(exception instanceof Exception)) { + return null; + } + try { + return provider.getExceptionProperties((Exception) exception); + } catch (Exception providerException) { + // Don't let a misbehaving provider mask the original failure. + LOGGER.warning("ExceptionPropertiesProvider threw while extracting properties: " + providerException); + return null; + } + } + + // Builds the single-line JSON payload that mirrors the protobuf TaskFailureDetails shape + // consumed by the Durable Task host extension. + private static String buildFailureDetailsJson(Throwable exception, ExceptionPropertiesProvider provider) { + StringBuilder sb = new StringBuilder(256); + appendFailure(sb, exception, provider, 0); + return sb.toString(); + } + + private static void appendFailure( + StringBuilder sb, + Throwable exception, + ExceptionPropertiesProvider provider, + int depth) { + sb.append('{'); + sb.append("\"errorType\":"); + appendString(sb, exception.getClass().getName()); + sb.append(",\"errorMessage\":"); + appendString(sb, exception.getMessage() != null ? exception.getMessage() : ""); + sb.append(",\"stackTrace\":"); + appendString(sb, getFullStackTrace(exception)); + sb.append(",\"isNonRetriable\":false"); + + Map properties = safeGetProperties(provider, exception); + if (properties != null && !properties.isEmpty()) { + sb.append(",\"properties\":"); + appendValue(sb, properties); + } + + Throwable cause = exception.getCause(); + if (cause != null && cause != exception && depth < MAX_INNER_FAILURE_DEPTH) { + sb.append(",\"innerFailure\":"); + appendFailure(sb, cause, provider, depth + 1); + } + + sb.append('}'); + } + + @SuppressWarnings("unchecked") + private static void appendValue(StringBuilder sb, Object value) { + if (value == null) { + sb.append("null"); + } else if (value instanceof String) { + appendString(sb, (String) value); + } else if (value instanceof Boolean) { + sb.append(((Boolean) value) ? "true" : "false"); + } else if (value instanceof Double || value instanceof Float) { + double d = ((Number) value).doubleValue(); + if (Double.isNaN(d) || Double.isInfinite(d)) { + appendString(sb, value.toString()); + } else { + sb.append(value.toString()); + } + } else if (value instanceof Number) { + sb.append(value.toString()); + } else if (value instanceof Map) { + sb.append('{'); + boolean first = true; + for (Map.Entry entry : ((Map) value).entrySet()) { + if (!first) { + sb.append(','); + } + first = false; + appendString(sb, String.valueOf(entry.getKey())); + sb.append(':'); + appendValue(sb, entry.getValue()); + } + sb.append('}'); + } else if (value instanceof Iterable) { + sb.append('['); + boolean first = true; + for (Object item : (Iterable) value) { + if (!first) { + sb.append(','); + } + first = false; + appendValue(sb, item); + } + sb.append(']'); + } else if (value instanceof Object[]) { + sb.append('['); + Object[] array = (Object[]) value; + for (int i = 0; i < array.length; i++) { + if (i > 0) { + sb.append(','); + } + appendValue(sb, array[i]); + } + sb.append(']'); + } else { + appendString(sb, value.toString()); + } + } + + private static void appendString(StringBuilder sb, String value) { + sb.append('"'); + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + switch (c) { + case '"': + sb.append("\\\""); + break; + case '\\': + sb.append("\\\\"); + break; + case '\n': + sb.append("\\n"); + break; + case '\r': + sb.append("\\r"); + break; + case '\t': + sb.append("\\t"); + break; + case '\b': + sb.append("\\b"); + break; + case '\f': + sb.append("\\f"); + break; + default: + if (c < 0x20) { + sb.append(String.format("\\u%04x", (int) c)); + } else { + sb.append(c); + } + break; + } + } + sb.append('"'); + } + + private static String getFullStackTrace(Throwable e) { + StackTraceElement[] elements = e.getStackTrace(); + StringBuilder sb = new StringBuilder(elements.length * 64); + for (StackTraceElement element : elements) { + sb.append("\tat ").append(element.toString()).append(System.lineSeparator()); + } + return sb.toString(); + } + + /** + * Internal exception whose message carries the serialized {@code TaskFailureDetails} JSON + * payload. It intentionally has no cause so the Java worker reports its message verbatim. + */ + private static final class StructuredActivityFailure extends RuntimeException { + private static final long serialVersionUID = 1L; + + StructuredActivityFailure(String message) { + super(message, null, false, false); + } + } +} diff --git a/azurefunctions/src/main/resources/META-INF/services/com.microsoft.azure.functions.internal.spi.middleware.Middleware b/azurefunctions/src/main/resources/META-INF/services/com.microsoft.azure.functions.internal.spi.middleware.Middleware index 0ba98d04..a7cf3add 100644 --- a/azurefunctions/src/main/resources/META-INF/services/com.microsoft.azure.functions.internal.spi.middleware.Middleware +++ b/azurefunctions/src/main/resources/META-INF/services/com.microsoft.azure.functions.internal.spi.middleware.Middleware @@ -1,2 +1,3 @@ com.microsoft.durabletask.azurefunctions.internal.middleware.OrchestrationMiddleware -com.microsoft.durabletask.azurefunctions.internal.middleware.EntityMiddleware \ No newline at end of file +com.microsoft.durabletask.azurefunctions.internal.middleware.EntityMiddleware +com.microsoft.durabletask.azurefunctions.internal.middleware.ActivityMiddleware \ No newline at end of file diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java new file mode 100644 index 00000000..0a072122 --- /dev/null +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -0,0 +1,286 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.microsoft.durabletask.azurefunctions.internal.middleware; + +import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareChain; +import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext; +import com.microsoft.durabletask.ExceptionPropertiesProvider; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link ActivityMiddleware}, covering exception reshaping, pass-through behavior, + * and the cross-class-loader SPI discovery that guards against the worker-thread regression. + */ +public class ActivityMiddlewareTest { + + private static final String ACTIVITY_TRIGGER = "DurableActivityTrigger"; + + /** A MiddlewareChain whose {@code doNext} throws the supplied exception. */ + private static MiddlewareChain throwingChain(Exception toThrow) { + return context -> { + throw toThrow; + }; + } + + /** A test exception representing a user's activity failure. */ + private static final class BusinessException extends Exception { + private static final long serialVersionUID = 1L; + + BusinessException(String message) { + super(message); + } + } + + private MiddlewareContext activityContext() { + MiddlewareContext context = mock(MiddlewareContext.class); + when(context.getParameterName(anyString())).thenReturn("input"); + return context; + } + + @BeforeEach + void resetBefore() { + ActivityMiddleware.resetProviderCacheForTesting(); + } + + @AfterEach + void resetAfter() { + ActivityMiddleware.resetProviderCacheForTesting(); + } + + @Test + @DisplayName("Reshapes a failing activity into structured TaskFailureDetails JSON when the " + + "provider yields properties") + void reshapesFailureWhenProviderYieldsProperties() { + ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + Map properties = new LinkedHashMap<>(); + properties.put("code", "E123"); + properties.put("count", 7); + return properties; + }); + + BusinessException original = new BusinessException("boom"); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(original))); + + // The original exception is replaced by a structured-failure carrier whose message is JSON. + assertNotSameInstance(original, thrown); + String message = thrown.getMessage(); + assertNotNull(message); + assertTrue(message.startsWith("{"), "message should be a JSON object, was: " + message); + assertTrue(message.contains("\"errorType\":\"" + BusinessException.class.getName() + "\""), + message); + assertTrue(message.contains("\"errorMessage\":\"boom\""), message); + assertTrue(message.contains("\"code\":\"E123\""), message); + assertTrue(message.contains("\"count\":7"), message); + } + + @Test + @DisplayName("Rethrows the original exception unchanged when the provider returns no properties") + void rethrowsOriginalWhenProviderReturnsEmpty() { + ActivityMiddleware.setProviderSupplierForTesting( + () -> exception -> Collections.emptyMap()); + + BusinessException original = new BusinessException("boom"); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(original))); + + assertSame(original, thrown); + } + + @Test + @DisplayName("Rethrows the original exception unchanged when the provider returns null") + void rethrowsOriginalWhenProviderReturnsNull() { + ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> null); + + BusinessException original = new BusinessException("boom"); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(original))); + + assertSame(original, thrown); + } + + @Test + @DisplayName("Rethrows the original exception unchanged when no provider is registered") + void rethrowsOriginalWhenNoProvider() { + ActivityMiddleware.setProviderSupplierForTesting(() -> null); + + BusinessException original = new BusinessException("boom"); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(original))); + + assertSame(original, thrown); + } + + @Test + @DisplayName("Rethrows the original exception unchanged when the provider itself throws") + void rethrowsOriginalWhenProviderThrows() { + ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + throw new IllegalStateException("provider is broken"); + }); + + BusinessException original = new BusinessException("boom"); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(original))); + + assertSame(original, thrown); + } + + @Test + @DisplayName("Does not invoke the provider for non-activity triggers") + void passesThroughNonActivityTrigger() throws Exception { + AtomicInteger providerCalls = new AtomicInteger(); + ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + providerCalls.incrementAndGet(); + return Collections.singletonMap("k", "v"); + }); + + MiddlewareContext context = mock(MiddlewareContext.class); + when(context.getParameterName(anyString())).thenReturn(null); // not an activity + MiddlewareChain chain = mock(MiddlewareChain.class); + ActivityMiddleware middleware = new ActivityMiddleware(); + + middleware.invoke(context, chain); + + verify(chain, times(1)).doNext(context); + assertEquals(0, providerCalls.get(), "provider must not be consulted for non-activities"); + } + + @Test + @DisplayName("Reshapes nested causes into a nested innerFailure payload") + void reshapesNestedCauses() { + ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + // Attach a property only to the outer exception so we can assert nesting shape. + if ("outer".equals(exception.getMessage())) { + return Collections.singletonMap("layer", "outer"); + } + return null; + }); + + BusinessException cause = new BusinessException("inner"); + Exception outer = new RuntimeException("outer", cause); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(outer))); + + String message = thrown.getMessage(); + assertNotNull(message); + assertTrue(message.contains("\"innerFailure\":{"), message); + assertTrue(message.contains("\"errorType\":\"" + BusinessException.class.getName() + "\""), + message); + assertTrue(message.contains("\"layer\":\"outer\""), message); + } + + // --- Cross-class-loader SPI discovery (the worker-thread regression guard) --- + + @Test + @DisplayName("discoverProvider returns null when no candidate class loader exposes the SPI file") + void discoverProviderReturnsNullWhenNoServiceFileVisible() { + ClassLoader blind = getClass().getClassLoader(); + assertNull(ActivityMiddleware.discoverProvider(new ClassLoader[] {blind})); + } + + @Test + @DisplayName("discoverProvider falls back past a provider-blind class loader to one that " + + "exposes the SPI file") + void discoverProviderFallsBackToClassLoaderThatSeesServiceFile() throws Exception { + ClassLoader blind = getClass().getClassLoader(); + URLClassLoader appLike = newClassLoaderExposingProvider(blind); + try { + // The first (blind) class loader mirrors the Azure Functions worker thread's context + // class loader, which cannot see the app's META-INF/services registration. Discovery + // must not stop there; it must fall back to the class loader that does. + ExceptionPropertiesProvider provider = + ActivityMiddleware.discoverProvider(new ClassLoader[] {blind, appLike}); + + assertNotNull(provider, "provider should be discovered via the fallback class loader"); + assertInstanceOf(ExceptionPropertiesProvider.class, provider); + assertEquals(TestExceptionPropertiesProvider.class.getName(), + provider.getClass().getName()); + } finally { + appLike.close(); + } + } + + @Test + @DisplayName("discoverProvider skips null and duplicate candidate class loaders") + void discoverProviderSkipsNullAndDuplicateCandidates() throws Exception { + ClassLoader blind = getClass().getClassLoader(); + URLClassLoader appLike = newClassLoaderExposingProvider(blind); + try { + ExceptionPropertiesProvider provider = ActivityMiddleware.discoverProvider( + new ClassLoader[] {null, blind, blind, appLike, appLike}); + assertNotNull(provider); + assertEquals(TestExceptionPropertiesProvider.class.getName(), + provider.getClass().getName()); + } finally { + appLike.close(); + } + } + + /** + * Builds a URLClassLoader that exposes a {@code META-INF/services} registration for + * {@link TestExceptionPropertiesProvider}. The provider class itself is loaded via the parent + * (so it resolves to the same {@link ExceptionPropertiesProvider} type), while the service file + * is served from this loader's own URL root — mirroring how an app jar carries its SPI file. + */ + private URLClassLoader newClassLoaderExposingProvider(ClassLoader parent) throws IOException { + Path root = Files.createTempDirectory("amw-spi-"); + root.toFile().deleteOnExit(); + Path servicesDir = root.resolve("META-INF").resolve("services"); + Files.createDirectories(servicesDir); + Path serviceFile = servicesDir.resolve(ExceptionPropertiesProvider.class.getName()); + Files.write(serviceFile, + (TestExceptionPropertiesProvider.class.getName() + System.lineSeparator()) + .getBytes(StandardCharsets.UTF_8)); + serviceFile.toFile().deleteOnExit(); + servicesDir.toFile().deleteOnExit(); + root.resolve("META-INF").toFile().deleteOnExit(); + + URL rootUrl = root.toUri().toURL(); + return new URLClassLoader(new URL[] {rootUrl}, parent); + } + + private static void assertNotSameInstance(Object unexpected, Object actual) { + assertFalse(unexpected == actual, + "expected a different instance than the original exception"); + } +} diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/TestExceptionPropertiesProvider.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/TestExceptionPropertiesProvider.java new file mode 100644 index 00000000..c594f12a --- /dev/null +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/TestExceptionPropertiesProvider.java @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +package com.microsoft.durabletask.azurefunctions.internal.middleware; + +import com.microsoft.durabletask.ExceptionPropertiesProvider; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Public {@link ExceptionPropertiesProvider} used only by {@link ActivityMiddlewareTest} to verify + * SPI discovery across class loaders. It must be {@code public} with a public no-arg constructor so + * {@link java.util.ServiceLoader} can instantiate it. + */ +public class TestExceptionPropertiesProvider implements ExceptionPropertiesProvider { + + @Override + public Map getExceptionProperties(Exception exception) { + Map properties = new LinkedHashMap<>(); + properties.put("discoveredVia", "serviceLoader"); + return properties; + } +} diff --git a/internal/durabletask-protobuf/PROTO_SOURCE_COMMIT_HASH b/internal/durabletask-protobuf/PROTO_SOURCE_COMMIT_HASH index dcc8b2f3..a981c689 100644 --- a/internal/durabletask-protobuf/PROTO_SOURCE_COMMIT_HASH +++ b/internal/durabletask-protobuf/PROTO_SOURCE_COMMIT_HASH @@ -1 +1 @@ -98e138452d57586e3109545b94055448f2f6cc24 \ No newline at end of file +3145f9337fca9de57d2f89a6ff6f07150d34f1c2 \ No newline at end of file diff --git a/internal/durabletask-protobuf/protos/orchestrator_service.proto b/internal/durabletask-protobuf/protos/orchestrator_service.proto index e7e12524..3d9194ac 100644 --- a/internal/durabletask-protobuf/protos/orchestrator_service.proto +++ b/internal/durabletask-protobuf/protos/orchestrator_service.proto @@ -377,7 +377,7 @@ message OrchestratorResponse { // Zero-based position of the current chunk within a chunked completion sequence. // This field is omitted for non-chunked completions. - google.protobuf.Int32Value chunkIndex = 9 [deprecated=true];; + google.protobuf.Int32Value chunkIndex = 9 [deprecated=true]; } message CreateInstanceRequest { From d0df7f894009239175b9812a519201b67e17c160 Mon Sep 17 00:00:00 2001 From: "naiyuantian@microsoft.com" Date: Thu, 16 Jul 2026 20:53:34 -0700 Subject: [PATCH 2/6] update middleware private --- .../middleware/ActivityMiddleware.java | 79 +++++++++++++------ .../middleware/ActivityMiddlewareTest.java | 53 ++++++++++--- 2 files changed, 98 insertions(+), 34 deletions(-) diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java index 5d6721cc..4f1087a8 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -47,10 +47,15 @@ public class ActivityMiddleware implements Middleware { private static volatile boolean providerLoaded = false; private static ExceptionPropertiesProvider cachedProvider; - // Visible for testing only. When non-null, this supplier replaces SPI discovery so unit tests - // can exercise the reshaping and pass-through behavior without registering a real provider. + // Test-only override. When non-null, this supplier replaces SPI discovery so tests can inject a + // provider (or {@code null}) without registering a real one. Set/cleared via reflection. private static Supplier providerSupplierOverride; + /** + * Runs the activity and, if it fails and a provider supplies custom properties, replaces the + * failure with a structured {@code TaskFailureDetails} JSON payload; otherwise the original + * exception is rethrown unchanged. Non-activity invocations pass straight through. + */ @Override public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exception { String parameterName = context.getParameterName(ACTIVITY_TRIGGER); @@ -78,6 +83,11 @@ public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exce } } + /** + * Lazily resolves and caches the {@link ExceptionPropertiesProvider}, using the test override + * when present and otherwise discovering it via SPI. The result (including {@code null}) is + * cached for the lifetime of the worker. + */ private static ExceptionPropertiesProvider getProvider() { if (!providerLoaded) { synchronized (PROVIDER_LOCK) { @@ -92,14 +102,12 @@ private static ExceptionPropertiesProvider getProvider() { return cachedProvider; } + /** + * Discovers the app-registered {@link ExceptionPropertiesProvider} via SPI, trying the thread + * context, middleware, and interface class loaders in turn (the worker thread's context loader + * may not see the app's {@code META-INF/services} registration). + */ private static ExceptionPropertiesProvider discoverProvider() { - // The provider is registered via SPI in the function app's jar. Depending on how the - // Azure Functions Java worker dispatches invocations, the thread context class loader may - // be the worker's class loader (which cannot see the app's META-INF/services registration) - // rather than the app class loader. Try several candidate class loaders and use the first - // one that yields a provider. The class loader that loaded this middleware is bundled with - // the app (durabletask-azure-functions is an app dependency), so it can see the app's SPI - // registration and is the most reliable fallback. return discoverProvider(new ClassLoader[] { Thread.currentThread().getContextClassLoader(), ActivityMiddleware.class.getClassLoader(), @@ -107,11 +115,12 @@ private static ExceptionPropertiesProvider discoverProvider() { }); } - // Visible for testing. Iterates the candidate class loaders in order and returns the first - // provider discovered via SPI, skipping nulls and duplicates. This is the seam that guards - // against the worker-thread class loader regression: discovery must not stop at the (possibly - // provider-blind) thread context class loader. - static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candidates) { + /** + * Returns the first {@link ExceptionPropertiesProvider} found by {@link ServiceLoader} across + * the given class loaders (nulls and duplicates skipped), or {@code null} if none is found. + * This is the seam that guards against the worker-thread class loader regression. + */ + private static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candidates) { ClassLoader previous = null; for (ClassLoader classLoader : candidates) { if (classLoader == null || classLoader == previous) { @@ -134,9 +143,11 @@ static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candidates) { return null; } - // Visible for testing. Overrides SPI discovery with the given supplier (may be {@code null} to - // simulate "no provider registered") and clears the cached provider so the next lookup re-runs. - static void setProviderSupplierForTesting(Supplier supplier) { + /** + * Test-only. Overrides SPI discovery with the given supplier ({@code null} simulates "no + * provider registered") and clears the cache so the next lookup re-runs. Invoked via reflection. + */ + private static void setProviderSupplierForTesting(Supplier supplier) { synchronized (PROVIDER_LOCK) { providerSupplierOverride = supplier; providerLoaded = false; @@ -144,9 +155,11 @@ static void setProviderSupplierForTesting(Supplier } } - // Visible for testing. Restores real SPI discovery and clears any cached provider so tests do - // not leak state into one another (the provider is cached in a static field). - static void resetProviderCacheForTesting() { + /** + * Test-only. Restores real SPI discovery and clears the cached provider so tests do not leak + * state into one another (the provider is cached in a static field). Invoked via reflection. + */ + private static void resetProviderCacheForTesting() { synchronized (PROVIDER_LOCK) { providerSupplierOverride = null; providerLoaded = false; @@ -154,6 +167,10 @@ static void resetProviderCacheForTesting() { } } + /** + * Unwraps reflective {@link InvocationTargetException} layers to reach the user exception that + * actually caused the activity to fail. + */ private static Throwable unwrap(Throwable e) { Throwable current = e; while (current instanceof InvocationTargetException && current.getCause() != null) { @@ -162,6 +179,11 @@ private static Throwable unwrap(Throwable e) { return current; } + /** + * Invokes the provider defensively, returning {@code null} if the failure is not an + * {@link Exception} or the provider itself throws, so a misbehaving provider never masks the + * original failure. + */ private static Map safeGetProperties( ExceptionPropertiesProvider provider, Throwable exception) { @@ -177,14 +199,20 @@ private static Map safeGetProperties( } } - // Builds the single-line JSON payload that mirrors the protobuf TaskFailureDetails shape - // consumed by the Durable Task host extension. + /** + * Builds the single-line JSON payload that mirrors the protobuf {@code TaskFailureDetails} shape + * consumed by the Durable Task host extension. + */ private static String buildFailureDetailsJson(Throwable exception, ExceptionPropertiesProvider provider) { StringBuilder sb = new StringBuilder(256); appendFailure(sb, exception, provider, 0); return sb.toString(); } + /** + * Recursively appends one failure level (error type/message/stack trace, any custom properties, + * and the cause as a nested {@code innerFailure}) to the JSON buffer. + */ private static void appendFailure( StringBuilder sb, Throwable exception, @@ -214,6 +242,11 @@ private static void appendFailure( sb.append('}'); } + /** + * Serializes a single property value as JSON, handling strings, booleans, numbers (non-finite + * doubles fall back to strings), maps, iterables, and arrays; anything else is written as its + * {@code toString()}. + */ @SuppressWarnings("unchecked") private static void appendValue(StringBuilder sb, Object value) { if (value == null) { @@ -270,6 +303,7 @@ private static void appendValue(StringBuilder sb, Object value) { } } + /** Appends {@code value} as a JSON string literal, escaping quotes, backslashes, and control characters. */ private static void appendString(StringBuilder sb, String value) { sb.append('"'); for (int i = 0; i < value.length(); i++) { @@ -308,6 +342,7 @@ private static void appendString(StringBuilder sb, String value) { sb.append('"'); } + /** Formats the throwable's stack trace as newline-separated {@code \tat ...} frames. */ private static String getFullStackTrace(Throwable e) { StackTraceElement[] elements = e.getStackTrace(); StringBuilder sb = new StringBuilder(elements.length * 64); diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java index 0a072122..0eaabef4 100644 --- a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Test; import java.io.IOException; +import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; @@ -20,6 +21,7 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -65,21 +67,48 @@ private MiddlewareContext activityContext() { return context; } + // --- Reflection bridges to ActivityMiddleware's private test seams --- + // The seams are private (they are not part of the middleware's API), so tests reach them via + // reflection rather than widening visibility. + + private static void setProviderSupplier(Supplier supplier) { + invokeStatic("setProviderSupplierForTesting", new Class[] {Supplier.class}, supplier); + } + + private static void resetProviderCache() { + invokeStatic("resetProviderCacheForTesting", new Class[] {}); + } + + private static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candidates) { + return (ExceptionPropertiesProvider) invokeStatic( + "discoverProvider", new Class[] {ClassLoader[].class}, (Object) candidates); + } + + private static Object invokeStatic(String name, Class[] paramTypes, Object... args) { + try { + Method method = ActivityMiddleware.class.getDeclaredMethod(name, paramTypes); + method.setAccessible(true); + return method.invoke(null, args); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException("Failed to invoke ActivityMiddleware." + name, e); + } + } + @BeforeEach void resetBefore() { - ActivityMiddleware.resetProviderCacheForTesting(); + resetProviderCache(); } @AfterEach void resetAfter() { - ActivityMiddleware.resetProviderCacheForTesting(); + resetProviderCache(); } @Test @DisplayName("Reshapes a failing activity into structured TaskFailureDetails JSON when the " + "provider yields properties") void reshapesFailureWhenProviderYieldsProperties() { - ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + setProviderSupplier(() -> exception -> { Map properties = new LinkedHashMap<>(); properties.put("code", "E123"); properties.put("count", 7); @@ -107,7 +136,7 @@ void reshapesFailureWhenProviderYieldsProperties() { @Test @DisplayName("Rethrows the original exception unchanged when the provider returns no properties") void rethrowsOriginalWhenProviderReturnsEmpty() { - ActivityMiddleware.setProviderSupplierForTesting( + setProviderSupplier( () -> exception -> Collections.emptyMap()); BusinessException original = new BusinessException("boom"); @@ -122,7 +151,7 @@ void rethrowsOriginalWhenProviderReturnsEmpty() { @Test @DisplayName("Rethrows the original exception unchanged when the provider returns null") void rethrowsOriginalWhenProviderReturnsNull() { - ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> null); + setProviderSupplier(() -> exception -> null); BusinessException original = new BusinessException("boom"); ActivityMiddleware middleware = new ActivityMiddleware(); @@ -136,7 +165,7 @@ void rethrowsOriginalWhenProviderReturnsNull() { @Test @DisplayName("Rethrows the original exception unchanged when no provider is registered") void rethrowsOriginalWhenNoProvider() { - ActivityMiddleware.setProviderSupplierForTesting(() -> null); + setProviderSupplier(() -> null); BusinessException original = new BusinessException("boom"); ActivityMiddleware middleware = new ActivityMiddleware(); @@ -150,7 +179,7 @@ void rethrowsOriginalWhenNoProvider() { @Test @DisplayName("Rethrows the original exception unchanged when the provider itself throws") void rethrowsOriginalWhenProviderThrows() { - ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + setProviderSupplier(() -> exception -> { throw new IllegalStateException("provider is broken"); }); @@ -167,7 +196,7 @@ void rethrowsOriginalWhenProviderThrows() { @DisplayName("Does not invoke the provider for non-activity triggers") void passesThroughNonActivityTrigger() throws Exception { AtomicInteger providerCalls = new AtomicInteger(); - ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + setProviderSupplier(() -> exception -> { providerCalls.incrementAndGet(); return Collections.singletonMap("k", "v"); }); @@ -186,7 +215,7 @@ void passesThroughNonActivityTrigger() throws Exception { @Test @DisplayName("Reshapes nested causes into a nested innerFailure payload") void reshapesNestedCauses() { - ActivityMiddleware.setProviderSupplierForTesting(() -> exception -> { + setProviderSupplier(() -> exception -> { // Attach a property only to the outer exception so we can assert nesting shape. if ("outer".equals(exception.getMessage())) { return Collections.singletonMap("layer", "outer"); @@ -215,7 +244,7 @@ void reshapesNestedCauses() { @DisplayName("discoverProvider returns null when no candidate class loader exposes the SPI file") void discoverProviderReturnsNullWhenNoServiceFileVisible() { ClassLoader blind = getClass().getClassLoader(); - assertNull(ActivityMiddleware.discoverProvider(new ClassLoader[] {blind})); + assertNull(discoverProvider(new ClassLoader[] {blind})); } @Test @@ -229,7 +258,7 @@ void discoverProviderFallsBackToClassLoaderThatSeesServiceFile() throws Exceptio // class loader, which cannot see the app's META-INF/services registration. Discovery // must not stop there; it must fall back to the class loader that does. ExceptionPropertiesProvider provider = - ActivityMiddleware.discoverProvider(new ClassLoader[] {blind, appLike}); + discoverProvider(new ClassLoader[] {blind, appLike}); assertNotNull(provider, "provider should be discovered via the fallback class loader"); assertInstanceOf(ExceptionPropertiesProvider.class, provider); @@ -246,7 +275,7 @@ void discoverProviderSkipsNullAndDuplicateCandidates() throws Exception { ClassLoader blind = getClass().getClassLoader(); URLClassLoader appLike = newClassLoaderExposingProvider(blind); try { - ExceptionPropertiesProvider provider = ActivityMiddleware.discoverProvider( + ExceptionPropertiesProvider provider = discoverProvider( new ClassLoader[] {null, blind, blind, appLike, appLike}); assertNotNull(provider); assertEquals(TestExceptionPropertiesProvider.class.getName(), From 26c98e326743eeb57e36b8544ba651e13576fd1c Mon Sep 17 00:00:00 2001 From: "naiyuantian@microsoft.com" Date: Sun, 19 Jul 2026 20:56:56 -0700 Subject: [PATCH 3/6] fx copilot comment --- .../middleware/ActivityMiddleware.java | 23 ++++++++++++------- .../middleware/ActivityMiddlewareTest.java | 11 +++++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java index 4f1087a8..84e6fc84 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.ServiceLoader; import java.util.function.Supplier; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -79,7 +80,7 @@ public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exce throw e; } - throw new StructuredActivityFailure(buildFailureDetailsJson(userException, provider)); + throw new StructuredActivityFailure(buildFailureDetailsJson(userException, properties, provider)); } } @@ -136,8 +137,9 @@ private static ExceptionPropertiesProvider discoverProvider(ClassLoader[] candid } } catch (Throwable t) { // Discovery failures must not break activity execution; the feature is opt-in. - LOGGER.warning("Failed to load ExceptionPropertiesProvider via ServiceLoader using " - + classLoader + ": " + t); + LOGGER.log(Level.WARNING, + "Failed to load ExceptionPropertiesProvider via ServiceLoader using " + classLoader, + t); } } return null; @@ -194,7 +196,9 @@ private static Map safeGetProperties( return provider.getExceptionProperties((Exception) exception); } catch (Exception providerException) { // Don't let a misbehaving provider mask the original failure. - LOGGER.warning("ExceptionPropertiesProvider threw while extracting properties: " + providerException); + LOGGER.log(Level.WARNING, + "ExceptionPropertiesProvider threw while extracting properties; ignoring provider output.", + providerException); return null; } } @@ -203,9 +207,12 @@ private static Map safeGetProperties( * Builds the single-line JSON payload that mirrors the protobuf {@code TaskFailureDetails} shape * consumed by the Durable Task host extension. */ - private static String buildFailureDetailsJson(Throwable exception, ExceptionPropertiesProvider provider) { + private static String buildFailureDetailsJson( + Throwable exception, + Map properties, + ExceptionPropertiesProvider provider) { StringBuilder sb = new StringBuilder(256); - appendFailure(sb, exception, provider, 0); + appendFailure(sb, exception, properties, provider, 0); return sb.toString(); } @@ -216,6 +223,7 @@ private static String buildFailureDetailsJson(Throwable exception, ExceptionProp private static void appendFailure( StringBuilder sb, Throwable exception, + Map properties, ExceptionPropertiesProvider provider, int depth) { sb.append('{'); @@ -227,7 +235,6 @@ private static void appendFailure( appendString(sb, getFullStackTrace(exception)); sb.append(",\"isNonRetriable\":false"); - Map properties = safeGetProperties(provider, exception); if (properties != null && !properties.isEmpty()) { sb.append(",\"properties\":"); appendValue(sb, properties); @@ -236,7 +243,7 @@ private static void appendFailure( Throwable cause = exception.getCause(); if (cause != null && cause != exception && depth < MAX_INNER_FAILURE_DEPTH) { sb.append(",\"innerFailure\":"); - appendFailure(sb, cause, provider, depth + 1); + appendFailure(sb, cause, safeGetProperties(provider, cause), provider, depth + 1); } sb.append('}'); diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java index 0eaabef4..e6711979 100644 --- a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -9,6 +9,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import java.io.IOException; import java.lang.reflect.Method; @@ -45,6 +46,10 @@ public class ActivityMiddlewareTest { private static final String ACTIVITY_TRIGGER = "DurableActivityTrigger"; + /** Auto-cleaned temp directory root for SPI class loader fixtures. */ + @TempDir + Path tempDir; + /** A MiddlewareChain whose {@code doNext} throws the supplied exception. */ private static MiddlewareChain throwingChain(Exception toThrow) { return context -> { @@ -292,17 +297,13 @@ void discoverProviderSkipsNullAndDuplicateCandidates() throws Exception { * is served from this loader's own URL root — mirroring how an app jar carries its SPI file. */ private URLClassLoader newClassLoaderExposingProvider(ClassLoader parent) throws IOException { - Path root = Files.createTempDirectory("amw-spi-"); - root.toFile().deleteOnExit(); + Path root = Files.createTempDirectory(tempDir, "amw-spi-"); Path servicesDir = root.resolve("META-INF").resolve("services"); Files.createDirectories(servicesDir); Path serviceFile = servicesDir.resolve(ExceptionPropertiesProvider.class.getName()); Files.write(serviceFile, (TestExceptionPropertiesProvider.class.getName() + System.lineSeparator()) .getBytes(StandardCharsets.UTF_8)); - serviceFile.toFile().deleteOnExit(); - servicesDir.toFile().deleteOnExit(); - root.resolve("META-INF").toFile().deleteOnExit(); URL rootUrl = root.toUri().toURL(); return new URLClassLoader(new URL[] {rootUrl}, parent); From b5b07518d38342a1ba4a3986582f67d5523fef48 Mon Sep 17 00:00:00 2001 From: Naiyuan Tian Date: Fri, 14 Aug 2026 07:11:03 -0700 Subject: [PATCH 4/6] add inner exception extract --- .../middleware/ActivityMiddleware.java | 27 +++++----- .../middleware/ActivityMiddlewareTest.java | 51 +++++++++++++++++++ .../microsoft/durabletask/FailureDetails.java | 2 +- .../durabletask/FailureDetailsTest.java | 25 +++++++++ 4 files changed, 91 insertions(+), 14 deletions(-) diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java index 84e6fc84..c5fd2559 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -23,8 +23,8 @@ * Durable Function Activity Middleware. * *

When an activity function throws, this middleware gives a registered - * {@link ExceptionPropertiesProvider} the chance to attach custom properties to the failure. If the - * provider returns any properties, the exception is reshaped into a serialized + * {@link ExceptionPropertiesProvider} the chance to attach custom properties to the failure or any + * exception in its causal chain. If the provider returns any properties, the exception is reshaped into a serialized * {@code TaskFailureDetails} JSON payload (matching the protobuf JSON shape) so the Durable Task * host extension can surface the structured properties on {@code FailureDetails.Properties}. This * mirrors the {@code durable-functions} JavaScript SDK's activity handler wrapper. @@ -74,13 +74,13 @@ public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exce } Throwable userException = unwrap(e); - Map properties = safeGetProperties(provider, userException); - if (properties == null || properties.isEmpty()) { - // No custom properties for this failure - preserve the original behavior. + String failureDetailsJson = buildFailureDetailsJson(userException, provider); + if (failureDetailsJson == null) { + // No custom properties for this failure chain - preserve the original behavior. throw e; } - throw new StructuredActivityFailure(buildFailureDetailsJson(userException, properties, provider)); + throw new StructuredActivityFailure(failureDetailsJson); } } @@ -209,23 +209,23 @@ private static Map safeGetProperties( */ private static String buildFailureDetailsJson( Throwable exception, - Map properties, ExceptionPropertiesProvider provider) { StringBuilder sb = new StringBuilder(256); - appendFailure(sb, exception, properties, provider, 0); - return sb.toString(); + return appendFailure(sb, exception, provider, 0) ? sb.toString() : null; } /** * Recursively appends one failure level (error type/message/stack trace, any custom properties, * and the cause as a nested {@code innerFailure}) to the JSON buffer. */ - private static void appendFailure( + private static boolean appendFailure( StringBuilder sb, Throwable exception, - Map properties, ExceptionPropertiesProvider provider, int depth) { + Map properties = safeGetProperties(provider, exception); + boolean hasCustomProperties = properties != null && !properties.isEmpty(); + sb.append('{'); sb.append("\"errorType\":"); appendString(sb, exception.getClass().getName()); @@ -241,12 +241,13 @@ private static void appendFailure( } Throwable cause = exception.getCause(); - if (cause != null && cause != exception && depth < MAX_INNER_FAILURE_DEPTH) { + if (cause != null && cause != exception && depth + 1 < MAX_INNER_FAILURE_DEPTH) { sb.append(",\"innerFailure\":"); - appendFailure(sb, cause, safeGetProperties(provider, cause), provider, depth + 1); + hasCustomProperties |= appendFailure(sb, cause, provider, depth + 1); } sb.append('}'); + return hasCustomProperties; } /** diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java index e6711979..1d8210dc 100644 --- a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -243,6 +243,57 @@ void reshapesNestedCauses() { assertTrue(message.contains("\"layer\":\"outer\""), message); } + /** Verifies that properties supplied only for an inner exception still produce structured failure details. */ + @Test + @DisplayName("Reshapes a failure when only an inner cause yields properties") + void reshapesFailureWhenOnlyInnerCauseYieldsProperties() { + setProviderSupplier(() -> exception -> { + if ("inner".equals(exception.getMessage())) { + return Collections.singletonMap("layer", "inner"); + } + return null; + }); + + BusinessException cause = new BusinessException("inner"); + Exception outer = new RuntimeException("outer", cause); + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(outer))); + + assertFalse(outer == thrown, "the inner provider properties should produce structured details"); + String message = thrown.getMessage(); + assertNotNull(message); + assertTrue(message.contains("\"innerFailure\":{"), message); + assertTrue(message.contains("\"layer\":\"inner\""), message); + } + + /** Verifies that the middleware stops provider invocations and failure serialization after ten levels. */ + @Test + @DisplayName("Limits exception provider calls to ten failure levels") + void limitsProviderCallsToTenFailureLevels() { + AtomicInteger providerCalls = new AtomicInteger(); + setProviderSupplier(() -> exception -> { + providerCalls.incrementAndGet(); + return Collections.singletonMap("level", exception.getMessage()); + }); + + Exception exception = new BusinessException("level 10"); + for (int level = 9; level >= 0; level--) { + exception = new RuntimeException("level " + level, exception); + } + Exception outer = exception; + ActivityMiddleware middleware = new ActivityMiddleware(); + + Exception thrown = assertThrows(Exception.class, + () -> middleware.invoke(activityContext(), throwingChain(outer))); + + assertEquals(10, providerCalls.get()); + String message = thrown.getMessage(); + assertNotNull(message); + assertFalse(message.contains("level 10"), message); + } + // --- Cross-class-loader SPI discovery (the worker-thread regression guard) --- @Test diff --git a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java index 93679851..fbd9e75c 100644 --- a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java +++ b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java @@ -229,7 +229,7 @@ private static FailureDetails fromExceptionRecursive( @Nullable Throwable exception, @Nullable ExceptionPropertiesProvider provider, int depth) { - if (exception == null || depth > MAX_INNER_FAILURE_DEPTH) { + if (exception == null || depth >= MAX_INNER_FAILURE_DEPTH) { return null; } Map properties = null; diff --git a/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java b/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java index ad3279a9..c10da0eb 100644 --- a/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java +++ b/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.jupiter.api.Assertions.*; @@ -171,6 +172,30 @@ void fromException_withProvider_extractsAndRoundTrips() { assertEquals("IOException", roundTripped.getInnerFailure().getProperties().get("exceptionType")); } + /** Verifies that recursive failure construction stops after ten levels and provider invocations. */ + @Test + void fromException_limitsProviderCallsToTenFailureLevels() { + AtomicInteger providerCalls = new AtomicInteger(); + ExceptionPropertiesProvider provider = exception -> { + providerCalls.incrementAndGet(); + return null; + }; + + Exception exception = new IOException("level 10"); + for (int level = 9; level >= 0; level--) { + exception = new RuntimeException("level " + level, exception); + } + + FailureDetails details = FailureDetails.fromException(exception, provider); + + assertEquals(10, providerCalls.get()); + int failureLevels = 0; + for (FailureDetails current = details; current != null; current = current.getInnerFailure()) { + failureLevels++; + } + assertEquals(10, failureLevels); + } + @Test void fromException_withNullProvider_noProperties() { RuntimeException ex = new RuntimeException("test", new IOException("cause")); From 6b21ad13ac8b11439bd60f6b80e818eb79bcf0fd Mon Sep 17 00:00:00 2001 From: Naiyuan Tian Date: Mon, 17 Aug 2026 13:10:37 -0700 Subject: [PATCH 5/6] update comments --- .../internal/middleware/ActivityMiddleware.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java index c5fd2559..7d510156 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -26,17 +26,12 @@ * {@link ExceptionPropertiesProvider} the chance to attach custom properties to the failure or any * exception in its causal chain. If the provider returns any properties, the exception is reshaped into a serialized * {@code TaskFailureDetails} JSON payload (matching the protobuf JSON shape) so the Durable Task - * host extension can surface the structured properties on {@code FailureDetails.Properties}. This - * mirrors the {@code durable-functions} JavaScript SDK's activity handler wrapper. - * + * host extension can surface the structured properties on {@code FailureDetails.Properties}. *

If no provider is registered, or it yields no properties for the thrown exception, the original - * exception is re-thrown untouched so the legacy failure behavior is preserved. + * exception is re-thrown untouched. * *

The provider is discovered via {@link ServiceLoader} (SPI): an application registers its * implementation in {@code META-INF/services/com.microsoft.durabletask.ExceptionPropertiesProvider}. - * - *

This class is internal and is hence not for public use. Its APIs are unstable and can change - * at any time. */ public class ActivityMiddleware implements Middleware { From d0b62cf6264b6b99421a14f29f2a1ddbc9337e6d Mon Sep 17 00:00:00 2001 From: Naiyuan Tian Date: Thu, 20 Aug 2026 09:54:54 -0700 Subject: [PATCH 6/6] address comments --- azurefunctions/build.gradle | 1 + .../middleware/ActivityMiddleware.java | 204 +++--------------- .../middleware/ActivityMiddlewareTest.java | 22 +- client/build.gradle | 1 + .../microsoft/durabletask/FailureDetails.java | 30 ++- .../durabletask/FailureDetailsTest.java | 18 ++ 6 files changed, 83 insertions(+), 193 deletions(-) diff --git a/azurefunctions/build.gradle b/azurefunctions/build.gradle index 5563b5bd..ac751281 100644 --- a/azurefunctions/build.gradle +++ b/azurefunctions/build.gradle @@ -37,6 +37,7 @@ dependencies { api project(':client') implementation group: 'com.microsoft.azure.functions', name: 'azure-functions-java-library', version: '3.2.3' implementation "com.google.protobuf:protobuf-java:${protocVersion}" + implementation "com.google.protobuf:protobuf-java-util:${protocVersion}" compileOnly "com.microsoft.azure.functions:azure-functions-java-spi:1.1.0" // Test dependencies diff --git a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java index 7d510156..306cdaea 100644 --- a/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -6,10 +6,13 @@ package com.microsoft.durabletask.azurefunctions.internal.middleware; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; import com.microsoft.azure.functions.internal.spi.middleware.Middleware; import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareChain; import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext; import com.microsoft.durabletask.ExceptionPropertiesProvider; +import com.microsoft.durabletask.FailureDetails; import java.lang.reflect.InvocationTargetException; import java.util.Iterator; @@ -36,7 +39,8 @@ public class ActivityMiddleware implements Middleware { private static final String ACTIVITY_TRIGGER = "DurableActivityTrigger"; - private static final int MAX_INNER_FAILURE_DEPTH = 10; + private static final JsonFormat.Printer FAILURE_DETAILS_JSON_PRINTER = + JsonFormat.printer().omittingInsignificantWhitespace(); private static final Logger LOGGER = Logger.getLogger(ActivityMiddleware.class.getName()); private static final Object PROVIDER_LOCK = new Object(); @@ -68,14 +72,21 @@ public void invoke(MiddlewareContext context, MiddlewareChain chain) throws Exce throw e; } - Throwable userException = unwrap(e); - String failureDetailsJson = buildFailureDetailsJson(userException, provider); - if (failureDetailsJson == null) { + FailureDetails failureDetails = FailureDetails.fromException(unwrap(e), provider); + if (!hasCustomProperties(failureDetails)) { // No custom properties for this failure chain - preserve the original behavior. throw e; } - throw new StructuredActivityFailure(failureDetailsJson); + try { + throw new StructuredActivityFailure( + FAILURE_DETAILS_JSON_PRINTER.print(failureDetails.toProto())); + } catch (InvalidProtocolBufferException serializationException) { + LOGGER.log(Level.WARNING, + "Failed to serialize structured failure details; rethrowing the original exception.", + serializationException); + throw e; + } } } @@ -176,183 +187,16 @@ private static Throwable unwrap(Throwable e) { return current; } - /** - * Invokes the provider defensively, returning {@code null} if the failure is not an - * {@link Exception} or the provider itself throws, so a misbehaving provider never masks the - * original failure. - */ - private static Map safeGetProperties( - ExceptionPropertiesProvider provider, - Throwable exception) { - if (!(exception instanceof Exception)) { - return null; - } - try { - return provider.getExceptionProperties((Exception) exception); - } catch (Exception providerException) { - // Don't let a misbehaving provider mask the original failure. - LOGGER.log(Level.WARNING, - "ExceptionPropertiesProvider threw while extracting properties; ignoring provider output.", - providerException); - return null; - } - } - - /** - * Builds the single-line JSON payload that mirrors the protobuf {@code TaskFailureDetails} shape - * consumed by the Durable Task host extension. - */ - private static String buildFailureDetailsJson( - Throwable exception, - ExceptionPropertiesProvider provider) { - StringBuilder sb = new StringBuilder(256); - return appendFailure(sb, exception, provider, 0) ? sb.toString() : null; - } - - /** - * Recursively appends one failure level (error type/message/stack trace, any custom properties, - * and the cause as a nested {@code innerFailure}) to the JSON buffer. - */ - private static boolean appendFailure( - StringBuilder sb, - Throwable exception, - ExceptionPropertiesProvider provider, - int depth) { - Map properties = safeGetProperties(provider, exception); - boolean hasCustomProperties = properties != null && !properties.isEmpty(); - - sb.append('{'); - sb.append("\"errorType\":"); - appendString(sb, exception.getClass().getName()); - sb.append(",\"errorMessage\":"); - appendString(sb, exception.getMessage() != null ? exception.getMessage() : ""); - sb.append(",\"stackTrace\":"); - appendString(sb, getFullStackTrace(exception)); - sb.append(",\"isNonRetriable\":false"); - - if (properties != null && !properties.isEmpty()) { - sb.append(",\"properties\":"); - appendValue(sb, properties); - } - - Throwable cause = exception.getCause(); - if (cause != null && cause != exception && depth + 1 < MAX_INNER_FAILURE_DEPTH) { - sb.append(",\"innerFailure\":"); - hasCustomProperties |= appendFailure(sb, cause, provider, depth + 1); - } - - sb.append('}'); - return hasCustomProperties; - } - - /** - * Serializes a single property value as JSON, handling strings, booleans, numbers (non-finite - * doubles fall back to strings), maps, iterables, and arrays; anything else is written as its - * {@code toString()}. - */ - @SuppressWarnings("unchecked") - private static void appendValue(StringBuilder sb, Object value) { - if (value == null) { - sb.append("null"); - } else if (value instanceof String) { - appendString(sb, (String) value); - } else if (value instanceof Boolean) { - sb.append(((Boolean) value) ? "true" : "false"); - } else if (value instanceof Double || value instanceof Float) { - double d = ((Number) value).doubleValue(); - if (Double.isNaN(d) || Double.isInfinite(d)) { - appendString(sb, value.toString()); - } else { - sb.append(value.toString()); - } - } else if (value instanceof Number) { - sb.append(value.toString()); - } else if (value instanceof Map) { - sb.append('{'); - boolean first = true; - for (Map.Entry entry : ((Map) value).entrySet()) { - if (!first) { - sb.append(','); - } - first = false; - appendString(sb, String.valueOf(entry.getKey())); - sb.append(':'); - appendValue(sb, entry.getValue()); - } - sb.append('}'); - } else if (value instanceof Iterable) { - sb.append('['); - boolean first = true; - for (Object item : (Iterable) value) { - if (!first) { - sb.append(','); - } - first = false; - appendValue(sb, item); - } - sb.append(']'); - } else if (value instanceof Object[]) { - sb.append('['); - Object[] array = (Object[]) value; - for (int i = 0; i < array.length; i++) { - if (i > 0) { - sb.append(','); - } - appendValue(sb, array[i]); - } - sb.append(']'); - } else { - appendString(sb, value.toString()); - } - } - - /** Appends {@code value} as a JSON string literal, escaping quotes, backslashes, and control characters. */ - private static void appendString(StringBuilder sb, String value) { - sb.append('"'); - for (int i = 0; i < value.length(); i++) { - char c = value.charAt(i); - switch (c) { - case '"': - sb.append("\\\""); - break; - case '\\': - sb.append("\\\\"); - break; - case '\n': - sb.append("\\n"); - break; - case '\r': - sb.append("\\r"); - break; - case '\t': - sb.append("\\t"); - break; - case '\b': - sb.append("\\b"); - break; - case '\f': - sb.append("\\f"); - break; - default: - if (c < 0x20) { - sb.append(String.format("\\u%04x", (int) c)); - } else { - sb.append(c); - } - break; + private static boolean hasCustomProperties(FailureDetails failureDetails) { + for (FailureDetails current = failureDetails; + current != null; + current = current.getInnerFailure()) { + Map properties = current.getProperties(); + if (properties != null && !properties.isEmpty()) { + return true; } } - sb.append('"'); - } - - /** Formats the throwable's stack trace as newline-separated {@code \tat ...} frames. */ - private static String getFullStackTrace(Throwable e) { - StackTraceElement[] elements = e.getStackTrace(); - StringBuilder sb = new StringBuilder(elements.length * 64); - for (StackTraceElement element : elements) { - sb.append("\tat ").append(element.toString()).append(System.lineSeparator()); - } - return sb.toString(); + return false; } /** diff --git a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java index 1d8210dc..1d21175b 100644 --- a/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -2,9 +2,12 @@ // Licensed under the MIT License. package com.microsoft.durabletask.azurefunctions.internal.middleware; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareChain; import com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext; import com.microsoft.durabletask.ExceptionPropertiesProvider; +import com.microsoft.durabletask.implementation.protobuf.OrchestratorService.TaskFailureDetails; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -28,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -112,11 +116,12 @@ void resetAfter() { @Test @DisplayName("Reshapes a failing activity into structured TaskFailureDetails JSON when the " + "provider yields properties") - void reshapesFailureWhenProviderYieldsProperties() { + void reshapesFailureWhenProviderYieldsProperties() throws InvalidProtocolBufferException { setProviderSupplier(() -> exception -> { Map properties = new LinkedHashMap<>(); properties.put("code", "E123"); properties.put("count", 7); + properties.put("attempts", new int[] {1, 2}); return properties; }); @@ -127,7 +132,7 @@ void reshapesFailureWhenProviderYieldsProperties() { () -> middleware.invoke(activityContext(), throwingChain(original))); // The original exception is replaced by a structured-failure carrier whose message is JSON. - assertNotSameInstance(original, thrown); + assertNotSame(original, thrown); String message = thrown.getMessage(); assertNotNull(message); assertTrue(message.startsWith("{"), "message should be a JSON object, was: " + message); @@ -136,6 +141,10 @@ void reshapesFailureWhenProviderYieldsProperties() { assertTrue(message.contains("\"errorMessage\":\"boom\""), message); assertTrue(message.contains("\"code\":\"E123\""), message); assertTrue(message.contains("\"count\":7"), message); + + TaskFailureDetails failureDetails = parseFailureDetails(message); + assertFalse(failureDetails.getIsNonRetriable()); + assertEquals(2, failureDetails.getPropertiesMap().get("attempts").getListValue().getValuesCount()); } @Test @@ -261,7 +270,7 @@ void reshapesFailureWhenOnlyInnerCauseYieldsProperties() { Exception thrown = assertThrows(Exception.class, () -> middleware.invoke(activityContext(), throwingChain(outer))); - assertFalse(outer == thrown, "the inner provider properties should produce structured details"); + assertNotSame(outer, thrown, "the inner provider properties should produce structured details"); String message = thrown.getMessage(); assertNotNull(message); assertTrue(message.contains("\"innerFailure\":{"), message); @@ -360,8 +369,9 @@ private URLClassLoader newClassLoaderExposingProvider(ClassLoader parent) throws return new URLClassLoader(new URL[] {rootUrl}, parent); } - private static void assertNotSameInstance(Object unexpected, Object actual) { - assertFalse(unexpected == actual, - "expected a different instance than the original exception"); + private static TaskFailureDetails parseFailureDetails(String json) throws InvalidProtocolBufferException { + TaskFailureDetails.Builder builder = TaskFailureDetails.newBuilder(); + JsonFormat.parser().merge(json, builder); + return builder.build(); } } diff --git a/client/build.gradle b/client/build.gradle index b7839ea3..d68b57de 100644 --- a/client/build.gradle +++ b/client/build.gradle @@ -39,6 +39,7 @@ def exeSuffix = isWindows ? ".exe" : "" dependencies { // https://github.com/grpc/grpc-java#download + api "com.google.protobuf:protobuf-java:${protocVersion}" implementation "io.grpc:grpc-protobuf:${grpcVersion}" implementation "io.grpc:grpc-stub:${grpcVersion}" runtimeOnly "io.grpc:grpc-netty-shaded:${grpcVersion}" diff --git a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java index fbd9e75c..3649e382 100644 --- a/client/src/main/java/com/microsoft/durabletask/FailureDetails.java +++ b/client/src/main/java/com/microsoft/durabletask/FailureDetails.java @@ -11,6 +11,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.lang.reflect.Array; import java.util.*; import java.util.logging.Level; import java.util.logging.Logger; @@ -71,7 +72,10 @@ public final class FailureDetails { * @param provider the provider for extracting custom properties, or {@code null} * @return a new {@code FailureDetails} instance */ - static FailureDetails fromException(Throwable throwable, @Nullable ExceptionPropertiesProvider provider) { + @Nonnull + public static FailureDetails fromException( + @Nonnull Throwable throwable, + @Nullable ExceptionPropertiesProvider provider) { return fromExceptionRecursive(throwable, provider, 0); } @@ -204,7 +208,13 @@ static String getFullStackTrace(Throwable e) { return sb.toString(); } - TaskFailureDetails toProto() { + /** + * Converts this failure to its protocol representation. + * + * @return the protocol representation of this failure + */ + @Nonnull + public TaskFailureDetails toProto() { TaskFailureDetails.Builder builder = TaskFailureDetails.newBuilder() .setErrorType(this.getErrorType()) .setErrorMessage(this.getErrorMessage()) @@ -306,7 +316,6 @@ private static Map convertToProtoProperties(Map p return result; } - @SuppressWarnings("unchecked") private static Value convertToProtoValue(@Nullable Object obj) { if (obj == null) { return Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(); @@ -316,16 +325,23 @@ private static Value convertToProtoValue(@Nullable Object obj) { return Value.newBuilder().setBoolValue((Boolean) obj).build(); } else if (obj instanceof String) { return Value.newBuilder().setStringValue((String) obj).build(); - } else if (obj instanceof List) { + } else if (obj instanceof Iterable) { ListValue.Builder listBuilder = ListValue.newBuilder(); - for (Object item : (List) obj) { + for (Object item : (Iterable) obj) { listBuilder.addValues(convertToProtoValue(item)); } return Value.newBuilder().setListValue(listBuilder).build(); + } else if (obj.getClass().isArray()) { + ListValue.Builder listBuilder = ListValue.newBuilder(); + int length = Array.getLength(obj); + for (int index = 0; index < length; index++) { + listBuilder.addValues(convertToProtoValue(Array.get(obj, index))); + } + return Value.newBuilder().setListValue(listBuilder).build(); } else if (obj instanceof Map) { Struct.Builder structBuilder = Struct.newBuilder(); - for (Map.Entry entry : ((Map) obj).entrySet()) { - structBuilder.putFields(entry.getKey(), convertToProtoValue(entry.getValue())); + for (Map.Entry entry : ((Map) obj).entrySet()) { + structBuilder.putFields(String.valueOf(entry.getKey()), convertToProtoValue(entry.getValue())); } return Value.newBuilder().setStructValue(structBuilder).build(); } else { diff --git a/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java b/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java index c10da0eb..280c45ed 100644 --- a/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java +++ b/client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java @@ -140,6 +140,24 @@ void toProto_roundTrip_withInnerFailureAndProperties() { assertNull(roundTrippedInner.getProperties().get("nullVal")); } + /** Verifies that primitive arrays are represented as protobuf lists rather than stringified values. */ + @Test + void toProto_primitiveArrayProperty_serializesAsList() { + Map properties = new HashMap<>(); + properties.put("attempts", new int[] {1, 2, 3}); + + FailureDetails details = new FailureDetails( + "CustomException", "error", "stack", false, null, properties); + Value attempts = details.toProto().getPropertiesMap().get("attempts"); + + assertNotNull(attempts); + assertEquals(Value.KindCase.LIST_VALUE, attempts.getKindCase()); + assertEquals(3, attempts.getListValue().getValuesCount()); + assertEquals(1.0, attempts.getListValue().getValues(0).getNumberValue()); + assertEquals(2.0, attempts.getListValue().getValues(1).getNumberValue()); + assertEquals(3.0, attempts.getListValue().getValues(2).getNumberValue()); + } + @Test void fromException_withProvider_extractsAndRoundTrips() { ExceptionPropertiesProvider provider = exception -> {