diff --git a/core/src/main/java/com/google/adk/workflow/RetryConfig.java b/core/src/main/java/com/google/adk/workflow/RetryConfig.java index 451d2f3eb..2a32467f1 100644 --- a/core/src/main/java/com/google/adk/workflow/RetryConfig.java +++ b/core/src/main/java/com/google/adk/workflow/RetryConfig.java @@ -72,7 +72,9 @@ public abstract class RetryConfig { /** * Returns the simple type names of the failures to retry on, matched exactly with no subclass - * match. When unset, any failure is retried; an empty list retries none. + * match. When unset, any failure is retried; an empty list retries none. A {@link + * WorkflowConfigurationException} is never retried, even when listed, because a retry fails the + * same way. */ public abstract Optional> exceptions(); @@ -92,7 +94,7 @@ public static Builder builder() { * from 1, has an attempt left and failed with a type this config retries. */ boolean shouldRetry(Exception failure, int attempt) { - if (attempt >= maxAttempts()) { + if (attempt >= maxAttempts() || failure instanceof WorkflowConfigurationException) { return false; } return exceptions().map(names -> names.contains(errorTypeName(failure))).orElse(true); diff --git a/core/src/test/java/com/google/adk/workflow/RetryConfigTest.java b/core/src/test/java/com/google/adk/workflow/RetryConfigTest.java index c563ff6db..824f172fe 100644 --- a/core/src/test/java/com/google/adk/workflow/RetryConfigTest.java +++ b/core/src/test/java/com/google/adk/workflow/RetryConfigTest.java @@ -181,6 +181,18 @@ public void shouldRetry_withAnEmptyExceptionsList_retriesNothing() { assertThat(config.shouldRetry(new IllegalStateException(), /* attempt= */ 1)).isFalse(); } + @Test + public void shouldRetry_neverRetriesAWorkflowConfigurationException() { + Exception failure = new WorkflowConfigurationException("misconfigured"); + RetryConfig listingIt = + RetryConfig.builder() + .exceptions(ImmutableList.of("WorkflowConfigurationException")) + .build(); + + assertThat(RetryConfig.builder().build().shouldRetry(failure, /* attempt= */ 1)).isFalse(); + assertThat(listingIt.shouldRetry(failure, /* attempt= */ 1)).isFalse(); + } + @Test public void delayFor_withoutJitter_growsByTheBackoffFactorUntilMaxDelay() { RetryConfig config = diff --git a/core/src/test/java/com/google/adk/workflow/WorkflowRetryTest.java b/core/src/test/java/com/google/adk/workflow/WorkflowRetryTest.java index ea4d84da2..62a1b91c4 100644 --- a/core/src/test/java/com/google/adk/workflow/WorkflowRetryTest.java +++ b/core/src/test/java/com/google/adk/workflow/WorkflowRetryTest.java @@ -17,6 +17,7 @@ package com.google.adk.workflow; import static com.google.adk.testing.WorkflowTestNodes.dispatcher; +import static com.google.adk.testing.WorkflowTestNodes.emitter; import static com.google.adk.testing.WorkflowTestNodes.throwing; import static com.google.adk.testing.WorkflowTestNodes.workflow; import static com.google.adk.workflow.WorkflowResumeTesting.START; @@ -383,6 +384,23 @@ public void anOuterTimeout_isNeitherBlamedOnNorRetriedByTheNodeItCancels() { assertThat(errorPaths(turn.events())).containsExactly("wf@1"); } + @Test + public void aWorkflowWithTwoTerminalOutputs_failsWithoutARetry() { + Workflow root = + Workflow.builder() + .name("wf") + .edges( + new Edge(START, emitter("first", "first_out")), + new Edge(START, emitter("second", "second_out"))) + .config(retrying(3)) + .build(); + + FailedTurn turn = new WorkflowSession(root).failingTurn(go()); + + assertThat(turn.error()).isInstanceOf(WorkflowConfigurationException.class); + assertThat(errorPaths(turn.events())).containsExactly("wf@1"); + } + @Test public void aWorkflowRetriedAfterANodeFailed_replaysTheNodesThatFinished() { CountingNode first = new CountingNode("first", (ctx, run) -> Flowable.just("first_out"));