From 2c675de3b6b5cf76db903e6ede902850db515fcd Mon Sep 17 00:00:00 2001 From: Mason Date: Fri, 6 Jun 2025 14:32:43 -0400 Subject: [PATCH 1/5] Add support to use both retry policy and handler at the same time Signed-off-by: Mason --- .../TaskOrchestrationExecutor.java | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index 569cc717..d47f1eab 100644 --- a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -406,10 +406,8 @@ public Task callSubOrchestrator( private Task createAppropriateTask(TaskFactory taskFactory, TaskOptions options) { // Retry policies and retry handlers will cause us to return a RetriableTask - if (options != null && options.hasRetryPolicy()) { - return new RetriableTask(this, taskFactory, options.getRetryPolicy()); - } if (options != null && options.hasRetryHandler()) { - return new RetriableTask(this, taskFactory, options.getRetryHandler()); + if (options != null && (options.hasRetryPolicy() || options.hasRetryHandler())) { + return new RetriableTask(this, taskFactory, options.getRetryPolicy(), options.getRetryHandler()); } else { // Return a single vanilla task without any wrapper return taskFactory.create(); @@ -1170,25 +1168,29 @@ private boolean shouldRetry() { return false; } - if (this.policy != null) { - logger.warning("Performing retires based on policy"); - - return this.shouldRetryBasedOnPolicy(); - } else if (this.handler != null) { - RetryContext retryContext = new RetryContext( - this.context, - this.attemptNumber, - this.lastFailure, - this.totalRetryTime); - return this.handler.handle(retryContext); - } else { + if(this.policy == null && this.handler == null) { // We should never get here, but if we do, returning false is the natural behavior. return false; } + + RetryContext retryContext = new RetryContext( + this.context, + this.attemptNumber, + this.lastFailure, + this.totalRetryTime); + + // These must default to true if not provided, so it is possible to use only one of them at a time + boolean shouldRetryBasedOnPolicy = this.policy == null || this.shouldRetryBasedOnPolicy(); + boolean shouldRetryBasedOnHandler = this.handler == null || this.handler.handle(retryContext); + + logger.info(() -> String.format("Retry policy provided: %s, shouldRetryBasedOnPolicy: %s", this.policy != null, shouldRetryBasedOnPolicy)); + logger.info(() -> String.format("Retry handler provided: %s, shouldRetryBasedOnHandler: %s", this.handler != null, shouldRetryBasedOnHandler)); + + return shouldRetryBasedOnPolicy && shouldRetryBasedOnHandler; } private boolean shouldRetryBasedOnPolicy() { - logger.warning(() -> String.format("%d retries out of total %d performed ", this.attemptNumber, this.policy.getMaxNumberOfAttempts())); + logger.warning(() -> String.format("Retry Policy: %d retries out of total %d performed ", this.attemptNumber, this.policy.getMaxNumberOfAttempts())); if (this.attemptNumber >= this.policy.getMaxNumberOfAttempts()) { // Max number of attempts exceeded From e9133702018af89f9281cae7d1a7c18cf3112626 Mon Sep 17 00:00:00 2001 From: Mason Date: Fri, 6 Jun 2025 14:45:37 -0400 Subject: [PATCH 2/5] Make TaskOptions constructor with both policy and handler public Signed-off-by: Mason --- client/src/main/java/io/dapr/durabletask/TaskOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/main/java/io/dapr/durabletask/TaskOptions.java b/client/src/main/java/io/dapr/durabletask/TaskOptions.java index cafb669c..50f47273 100644 --- a/client/src/main/java/io/dapr/durabletask/TaskOptions.java +++ b/client/src/main/java/io/dapr/durabletask/TaskOptions.java @@ -9,7 +9,7 @@ public final class TaskOptions { private final RetryPolicy retryPolicy; private final RetryHandler retryHandler; - private TaskOptions(RetryPolicy retryPolicy, RetryHandler retryHandler) { + public TaskOptions(RetryPolicy retryPolicy, RetryHandler retryHandler) { this.retryPolicy = retryPolicy; this.retryHandler = retryHandler; } From 9296158eed9b30a5502066fd0307fa299bde9703 Mon Sep 17 00:00:00 2001 From: Mason Date: Fri, 6 Jun 2025 14:51:18 -0400 Subject: [PATCH 3/5] Bump version to 1.5.6 Signed-off-by: Mason --- client/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/build.gradle b/client/build.gradle index 2d8cfdc8..26b3c2ba 100644 --- a/client/build.gradle +++ b/client/build.gradle @@ -10,7 +10,7 @@ plugins { } group 'io.dapr' -version = '1.5.5' +version = '1.5.6' archivesBaseName = 'durabletask-client' def grpcVersion = '1.69.0' From f0e839bd829a38d1c5925785b942dc7ae91d5bca Mon Sep 17 00:00:00 2001 From: Mason Date: Mon, 9 Jun 2025 13:45:53 -0400 Subject: [PATCH 4/5] Log only if policy or handler is present Signed-off-by: Mason --- .../io/dapr/durabletask/TaskOrchestrationExecutor.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index d47f1eab..0a40c72b 100644 --- a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -1183,8 +1183,13 @@ private boolean shouldRetry() { boolean shouldRetryBasedOnPolicy = this.policy == null || this.shouldRetryBasedOnPolicy(); boolean shouldRetryBasedOnHandler = this.handler == null || this.handler.handle(retryContext); - logger.info(() -> String.format("Retry policy provided: %s, shouldRetryBasedOnPolicy: %s", this.policy != null, shouldRetryBasedOnPolicy)); - logger.info(() -> String.format("Retry handler provided: %s, shouldRetryBasedOnHandler: %s", this.handler != null, shouldRetryBasedOnHandler)); + if (this.policy != null) { + logger.info(() -> String.format("shouldRetryBasedOnPolicy: %s", shouldRetryBasedOnPolicy)); + } + + if (this.handler != null) { + logger.info(() -> String.format("shouldRetryBasedOnHandler: %s", shouldRetryBasedOnHandler)); + } return shouldRetryBasedOnPolicy && shouldRetryBasedOnHandler; } From d7dcc56a7c0e7ff975b36cfe8e869b1fe05bfb32 Mon Sep 17 00:00:00 2001 From: Mason Date: Wed, 11 Jun 2025 07:59:25 -0400 Subject: [PATCH 5/5] Switch to ternary statement Co-authored-by: Javier Aliaga Signed-off-by: Mason --- .../java/io/dapr/durabletask/TaskOrchestrationExecutor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java index 0a40c72b..213c77f2 100644 --- a/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java +++ b/client/src/main/java/io/dapr/durabletask/TaskOrchestrationExecutor.java @@ -1180,8 +1180,8 @@ private boolean shouldRetry() { this.totalRetryTime); // These must default to true if not provided, so it is possible to use only one of them at a time - boolean shouldRetryBasedOnPolicy = this.policy == null || this.shouldRetryBasedOnPolicy(); - boolean shouldRetryBasedOnHandler = this.handler == null || this.handler.handle(retryContext); + boolean shouldRetryBasedOnPolicy = this.policy != null ? this.shouldRetryBasedOnPolicy() : true; + boolean shouldRetryBasedOnHandler = this.handler != null ? this.handler.handle(retryContext) : true; if (this.policy != null) { logger.info(() -> String.format("shouldRetryBasedOnPolicy: %s", shouldRetryBasedOnPolicy));