diff --git a/azurefunctions/build.gradle b/azurefunctions/build.gradle index 4d991038..ac751281 100644 --- a/azurefunctions/build.gradle +++ b/azurefunctions/build.gradle @@ -37,9 +37,11 @@ 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 + 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..306cdaea --- /dev/null +++ b/azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddleware.java @@ -0,0 +1,213 @@ +/** + * 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.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; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.function.Supplier; +import java.util.logging.Level; +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 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}. + *

If no provider is registered, or it yields no properties for the thrown exception, the original + * 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}. + */ +public class ActivityMiddleware implements Middleware { + + private static final String ACTIVITY_TRIGGER = "DurableActivityTrigger"; + 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(); + private static volatile boolean providerLoaded = false; + private static ExceptionPropertiesProvider cachedProvider; + + // 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); + if (parameterName == null) { + chain.doNext(context); + return; + } + + try { + chain.doNext(context); + } catch (Exception e) { + ExceptionPropertiesProvider provider = getProvider(); + if (provider == null) { + throw e; + } + + FailureDetails failureDetails = FailureDetails.fromException(unwrap(e), provider); + if (!hasCustomProperties(failureDetails)) { + // No custom properties for this failure chain - preserve the original behavior. + throw e; + } + + 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; + } + } + } + + /** + * 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) { + if (!providerLoaded) { + cachedProvider = providerSupplierOverride != null + ? providerSupplierOverride.get() + : discoverProvider(); + providerLoaded = true; + } + } + } + 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() { + return discoverProvider(new ClassLoader[] { + Thread.currentThread().getContextClassLoader(), + ActivityMiddleware.class.getClassLoader(), + ExceptionPropertiesProvider.class.getClassLoader(), + }); + } + + /** + * 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) { + 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.log(Level.WARNING, + "Failed to load ExceptionPropertiesProvider via ServiceLoader using " + classLoader, + t); + } + } + return null; + } + + /** + * 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; + cachedProvider = null; + } + } + + /** + * 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; + cachedProvider = null; + } + } + + /** + * 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) { + current = current.getCause(); + } + return current; + } + + 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; + } + } + return false; + } + + /** + * 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..1d21175b --- /dev/null +++ b/azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/internal/middleware/ActivityMiddlewareTest.java @@ -0,0 +1,377 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// 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; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.lang.reflect.Method; +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 java.util.function.Supplier; + +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.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; +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"; + + /** 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 -> { + 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; + } + + // --- 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() { + resetProviderCache(); + } + + @AfterEach + void resetAfter() { + resetProviderCache(); + } + + @Test + @DisplayName("Reshapes a failing activity into structured TaskFailureDetails JSON when the " + + "provider yields properties") + 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; + }); + + 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. + assertNotSame(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); + + TaskFailureDetails failureDetails = parseFailureDetails(message); + assertFalse(failureDetails.getIsNonRetriable()); + assertEquals(2, failureDetails.getPropertiesMap().get("attempts").getListValue().getValuesCount()); + } + + @Test + @DisplayName("Rethrows the original exception unchanged when the provider returns no properties") + void rethrowsOriginalWhenProviderReturnsEmpty() { + setProviderSupplier( + () -> 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() { + setProviderSupplier(() -> 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() { + setProviderSupplier(() -> 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() { + setProviderSupplier(() -> 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(); + setProviderSupplier(() -> 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() { + 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"); + } + 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); + } + + /** 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))); + + assertNotSame(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 + @DisplayName("discoverProvider returns null when no candidate class loader exposes the SPI file") + void discoverProviderReturnsNullWhenNoServiceFileVisible() { + ClassLoader blind = getClass().getClassLoader(); + assertNull(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 = + 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 = 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(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)); + + URL rootUrl = root.toUri().toURL(); + return new URLClassLoader(new URL[] {rootUrl}, parent); + } + + private static TaskFailureDetails parseFailureDetails(String json) throws InvalidProtocolBufferException { + TaskFailureDetails.Builder builder = TaskFailureDetails.newBuilder(); + JsonFormat.parser().merge(json, builder); + return builder.build(); + } +} 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/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 93679851..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()) @@ -229,7 +239,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; @@ -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 ad3279a9..280c45ed 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.*; @@ -139,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 -> { @@ -171,6 +190,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"));