Fix the issue with retries not happening correctly for Activities and Workflows (#1343) (#1374)

cicoyle · siri-varma · mcruzdev · web-flow · commit 868082cd7a8f · 2025-05-15T19:03:57.000-05:00
* Add coverage for some properties (#1297) * Make the DAPR version being used consistent across all tests (#1299) * Separate Dapr constants from IT container constants (#1315) * Use Java Bean for connection details and add more tests (#1317) * Use Java Bean for connection details and add more tests * Simplify mock setup * Adding even more tests for test coverage --------- * Update CONTRIBUTING.md * Bump codecov/codecov-action from 5.4.0 to 5.4.2 (#1318) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.4.0 to 5.4.2. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5.4.0...v5.4.2) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 5.4.2 dependency-type: direct:production update-type: version-update:semver-patch ... * Fix URL building logic (#1320) * Fix URL building logic * Add test for query params * Fix the assertion in the test * Adjust the tests * Remove uneeded changes from IT test * Revert some unintended changes * Simplify the testing a little bit * Adjust the test to use ServerRequest * Test removing things from method invoke controller * Add query param encoding test * Revert some unintended changes * Some tiny styles --------- * Generate updated javadocs for 1.14.1 * Add Conversation AI to Java SDK (#1235) * Conversation first commit * Add unit tests * change ai to conv * Move to single module * Remove module * Add Integration tests * Update sdk-tests/src/test/java/io/dapr/it/testcontainers/DaprConversationIT.java * Fix things * Address comments * Import tag * Address comments * Make common config * Address comments * fix constant * fix constant * fix constant * fix s * Fix things * Fix things * Fix things * Make common config * Update README.md * Update README.md --------- * Add docs for usage of Jobs SDK (#1323) * Add doc for jobs * Add docs for Jobs * Apply suggestions from code review --------- * Use dapr/durabletask-java (#1336) * microsoft durabletask-java -> dapr durabletask-java * update another ref * 1.5.2 release * fix import order * Sdk new changes * Refine workflows * add ; * rm try --------- * Update master version to 1.16.0-SNAPSHOT * Fix NPE * Fix NPE * Fix NPE * Fix NPE * Fix NPE * Fix NPE * Fix things * Renaming and exposing connection details (#1341) * [Master] Fix Vulnerabilities (#1354) * update okio * rm unused dep --------- * Feat Add TLS & mTLS support for gRPC with root CA and insecure mode (#1361) * feat: Support for GRPC ssl * add tests * fix CI * add back else if * channel cleanup * add root ca support * checkstyles * add insecure * fix checkstyles * use InsecureTrustManagerFactory * fix test --------- * Address comments * Fix things * Fix things --------- Signed-off-by: sirivarma <siri.varma@outlook.com> Signed-off-by: Artur Ciocanu <ciocanu@adobe.com> Signed-off-by: Siri Varma Vegiraju <siri.varma@outlook.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Dapr Bot <daprweb@microsoft.com> Signed-off-by: Siri Varma Vegiraju <svegiraju@microsoft.com> Signed-off-by: siri-varma <siri.varma@outlook.com> Signed-off-by: Cassandra Coyle <cassie@diagrid.io> Signed-off-by: Javier Aliaga <javier@diagrid.io> Co-authored-by: Siri Varma Vegiraju <siri.varma@outlook.com> Co-authored-by: Matheus Cruz <56329339+mcruzdev@users.noreply.github.com> Co-authored-by: artur-ciocanu <artur.ciocanu@gmail.com> Co-authored-by: Artur Ciocanu <ciocanu@adobe.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Co-authored-by: Dapr Bot <daprweb@microsoft.com> Co-authored-by: Javier Aliaga <javier@diagrid.io>
diff --git a/examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflow.java b/examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflow.java
@@ -15,18 +15,29 @@
 
 import io.dapr.workflows.Workflow;
 import io.dapr.workflows.WorkflowStub;
+import io.dapr.workflows.WorkflowTaskOptions;
+import io.dapr.workflows.WorkflowTaskRetryPolicy;
+
+import java.time.Duration;
 
 public class DemoChildWorkflow implements Workflow {
   @Override
   public WorkflowStub create() {
     return ctx -> {
       ctx.getLogger().info("Starting ChildWorkflow: " + ctx.getName());
 
+      WorkflowTaskRetryPolicy policy = WorkflowTaskRetryPolicy.newBuilder()
+              .setFirstRetryInterval(Duration.ofSeconds(1))
+              .setMaxNumberOfAttempts(10)
+              .build();
+
+      WorkflowTaskOptions options = new WorkflowTaskOptions(policy);
+
       var childWorkflowInput = ctx.getInput(String.class);
       ctx.getLogger().info("ChildWorkflow received input: " + childWorkflowInput);
 
       ctx.getLogger().info("ChildWorkflow is calling Activity: " + ReverseActivity.class.getName());
-      String result = ctx.callActivity(ReverseActivity.class.getName(), childWorkflowInput, String.class).await();
+      String result = ctx.callActivity(ReverseActivity.class.getName(), childWorkflowInput, options, String.class).await();
 
       ctx.getLogger().info("ChildWorkflow finished with: " + result);
       ctx.complete(result);
diff --git a/examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflowWorker.java b/examples/src/main/java/io/dapr/examples/workflows/childworkflow/DemoChildWorkflowWorker.java
@@ -32,6 +32,7 @@ public static void main(String[] args) throws Exception {
 
     // Build and then start the workflow runtime pulling and executing tasks
     WorkflowRuntime runtime = builder.build();
+    runtime.start();
     System.out.println("Start workflow runtime");
   }
 }
diff --git a/sdk-workflows/pom.xml b/sdk-workflows/pom.xml
@@ -47,7 +47,7 @@
     <dependency>
       <groupId>io.dapr</groupId>
       <artifactId>durabletask-client</artifactId>
-      <version>1.5.2</version>
+      <version>1.5.3</version>
     </dependency>
     <!--
     manually declare durabletask-client's jackson dependencies
diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java b/sdk-workflows/src/main/java/io/dapr/workflows/WorkflowTaskRetryPolicy.java
@@ -166,9 +166,10 @@ public Builder setMaxRetryInterval(@Nullable Duration maxRetryInterval) {
      * @return This builder
      */
     public Builder setRetryTimeout(Duration retryTimeout) {
-      if (retryTimeout != null && retryTimeout.compareTo(this.firstRetryInterval) < 0) {
+      if (retryTimeout == null || retryTimeout.compareTo(this.firstRetryInterval) < 0) {
         throw new IllegalArgumentException(
-            "The value for retryTimeout must be greater than or equal to the value for firstRetryInterval.");
+            "The value for retryTimeout cannot be null and"
+                    + " must be greater than or equal to the value for firstRetryInterval.");
       }
 
       this.retryTimeout = retryTimeout;
diff --git a/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java b/sdk-workflows/src/main/java/io/dapr/workflows/runtime/DefaultWorkflowContext.java
@@ -240,7 +240,9 @@ private static TaskOptions toTaskOptions(WorkflowTaskOptions options) {
     );
 
     retryPolicy.setBackoffCoefficient(workflowTaskRetryPolicy.getBackoffCoefficient());
-    retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());
+    if (workflowTaskRetryPolicy.getRetryTimeout() != null) {
+      retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());
+    }
 
     return new TaskOptions(retryPolicy);
   }
diff --git a/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java b/sdk-workflows/src/test/java/io/dapr/workflows/DefaultWorkflowContextTest.java
@@ -304,6 +304,7 @@ public void callChildWorkflowWithOptions() {
 
     assertEquals(retryPolicy.getMaxNumberOfAttempts(), taskOptions.getRetryPolicy().getMaxNumberOfAttempts());
     assertEquals(retryPolicy.getFirstRetryInterval(), taskOptions.getRetryPolicy().getFirstRetryInterval());
+    assertEquals(Duration.ZERO, taskOptions.getRetryPolicy().getRetryTimeout());
   }
 
   @Test
@@ -327,4 +328,52 @@ public void newUuidTestNoImplementationExceptionTest() {
     String expectedMessage = "No implementation found.";
     assertEquals(expectedMessage, runtimeException.getMessage());
   }
+
+  @Test
+  public void workflowRetryPolicyRetryTimeoutValueShouldHaveRightValueWhenBeingSet() {
+    String expectedName = "TestActivity";
+    String expectedInput = "TestInput";
+    String expectedInstanceId = "TestInstanceId";
+    WorkflowTaskRetryPolicy retryPolicy = WorkflowTaskRetryPolicy.newBuilder()
+            .setMaxNumberOfAttempts(1)
+            .setFirstRetryInterval(Duration.ofSeconds(10))
+            .setRetryTimeout(Duration.ofSeconds(10))
+            .build();
+    WorkflowTaskOptions executionOptions = new WorkflowTaskOptions(retryPolicy);
+    ArgumentCaptor<TaskOptions> captor = ArgumentCaptor.forClass(TaskOptions.class);
+
+    context.callChildWorkflow(expectedName, expectedInput, expectedInstanceId, executionOptions, String.class);
+
+    verify(mockInnerContext, times(1))
+            .callSubOrchestrator(
+                    eq(expectedName),
+                    eq(expectedInput),
+                    eq(expectedInstanceId),
+                    captor.capture(),
+                    eq(String.class)
+            );
+
+    TaskOptions taskOptions = captor.getValue();
+
+    assertEquals(Duration.ofSeconds(10), taskOptions.getRetryPolicy().getRetryTimeout());
+  }
+
+  @Test
+  public void workflowRetryPolicyRetryThrowIllegalArgumentWhenNullRetryTimeoutIsSet() {
+    assertThrows(IllegalArgumentException.class, () ->
+            WorkflowTaskRetryPolicy.newBuilder()
+                    .setMaxNumberOfAttempts(1)
+                    .setFirstRetryInterval(Duration.ofSeconds(10))
+                    .setRetryTimeout(null)
+                    .build());
+  }
+
+  @Test
+  public void workflowRetryPolicyRetryThrowIllegalArgumentWhenRetryTimeoutIsLessThanMaxRetryInterval() {
+    assertThrows(IllegalArgumentException.class, () -> WorkflowTaskRetryPolicy.newBuilder()
+            .setMaxNumberOfAttempts(1)
+            .setFirstRetryInterval(Duration.ofSeconds(10))
+            .setRetryTimeout(Duration.ofSeconds(9))
+            .build());
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ public static void main(String[] args) throws Exception {`
`32`	`32`
`33`	`33`	`// Build and then start the workflow runtime pulling and executing tasks`
`34`	`34`	`WorkflowRuntime runtime = builder.build();`
	`35`	`+ runtime.start();`
`35`	`36`	`System.out.println("Start workflow runtime");`
`36`	`37`	`}`
`37`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -240,7 +240,9 @@ private static TaskOptions toTaskOptions(WorkflowTaskOptions options) {`
`240`	`240`	`);`
`241`	`241`
`242`	`242`	`retryPolicy.setBackoffCoefficient(workflowTaskRetryPolicy.getBackoffCoefficient());`
`243`		`- retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());`
	`243`	`+ if (workflowTaskRetryPolicy.getRetryTimeout() != null) {`
	`244`	`+ retryPolicy.setRetryTimeout(workflowTaskRetryPolicy.getRetryTimeout());`
	`245`	`+ }`
`244`	`246`
`245`	`247`	`return new TaskOptions(retryPolicy);`
`246`	`248`	`}`