Implement retry attempt.duration per-attempt timeout

[mcruzdev]

Add support for attempt.duration per attempt timeout.

Closes #1526

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

[Copilot]

Pull request overview

This PR adds support for a per-retry-attempt timeout (limit.attempt.duration) in try/catch retry handling, and includes a workflow sample plus a regression test to validate the behavior.

Changes:

• Add attempt.duration parsing/resolution from retry policy and enforce it via CompletableFuture.orTimeout(...) during try execution.
• Add a new workflow sample exercising attempt.duration in a try/catch/retry block.
• Add a unit test verifying that a timed-out attempt retries and eventually succeeds.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java	Implements per-attempt timeout handling for retry attempts in TryExecutor.
impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java	Adds a test that simulates a slow HTTP response to trigger attempt.duration timeout and verify retry behavior.
impl/test/src/test/resources/workflows-samples/try-catch-retry-attempt-duration.yaml	Adds a workflow sample defining limit.attempt.duration under retry policy.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:196

• attempt.duration uses orTimeout(timeout.toMillis(), MILLISECONDS) guarded only by !timeout.isZero(). For positive sub-millisecond durations (possible via Duration.parse in expression/literal), timeout.toMillis() becomes 0 and orTimeout(0, …) throws IllegalArgumentException, bypassing the intended timeout/retry behavior. Also, negative durations currently fall through to orTimeout and would throw as well. Guard on timeout > 0 and ensure the millis value is at least 1 when applying the timeout.

      if (!timeout.isZero()) {
        future =
            future
                .orTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS)

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:110

• When resolving a retry policy by reference, this code can throw a NullPointerException if the workflow omits the use section or use.retries. Other parts of the codebase (e.g., WorkflowUtils.getTaskTimeout) use explicit null checks with a descriptive message; doing the same here would make misconfigurations much easier to diagnose.

      } else if (retry.getRetryPolicyReference() != null) {
        RetryPolicy retryPolicy =
            workflow
                .getUse()
                .getRetries()
                .getAdditionalProperties()
                .get(retry.getRetryPolicyReference());

[fjtirado]

Refactor using optional rather than null

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:214

• In the non-timeout path, this returns failedFuture(e) even though cause has already been unwrapped from CompletionException. That can re-wrap non-timeout exceptions and may interfere with later handling (e.g., handleException seeing a CompletionException instead of the original cause). Return cause (or rethrow cause) in the : ... branch to preserve the original exception.

  private CompletableFuture<WorkflowModel> handleTimeoutException(
      Throwable e, TaskContext taskContext) {
    Throwable cause = e instanceof CompletionException ? e.getCause() : e;
    return CompletableFuture.failedFuture(
        cause instanceof TimeoutException
            ? new WorkflowException(
                WorkflowError.timeout()
                    .instance(taskContext.position().jsonPointer())
                    .title(cause.getMessage())
                    .build(),
                cause)
            : e);

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:197

• Using a 2-second delayed response in a unit test can unnecessarily slow the suite and can also extend teardown if the server is still serving the delayed response. Since the workflow timeout in the sample is 50ms, consider reducing the delay to a smaller value that is still comfortably above the configured attempt timeout (e.g., a few hundred milliseconds) to keep the test fast and less flaky.

    apiServer.enqueue(
        new MockResponse()
            .setHeadersDelay(2, TimeUnit.SECONDS)
            .setResponseCode(200)
            .setHeader("Content-Type", "application/json")
            .setBody("{}"));

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:228

• In handleTimeoutException(), the non-timeout branch returns the original throwable e instead of the unwrapped cause. This can leak an extra CompletionException wrapper and interfere with downstream error handling that expects the root cause.

            : e);

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:109

• resolveRetryPolicy() can throw a NullPointerException when a retry policy reference is used but workflow.use or workflow.use.retries is not defined. This should fail fast with a clear IllegalStateException, consistent with other reference-resolution code (e.g., WorkflowUtils.getTaskTimeout).

        RetryPolicy retryPolicy =
            workflow
                .getUse()
                .getRetries()
                .getAdditionalProperties()

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:206

• testAttemptDuration() only asserts that a WorkflowException is thrown, but doesn’t verify it is specifically the new attempt-duration timeout behavior. Adding an assertion on the WorkflowError status/type would make this test catch regressions where the failure is caused by something else.

        .hasCauseInstanceOf(WorkflowException.class);

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:224

• Timeout errors created here set the WorkflowError title from the underlying TimeoutException message. JDK TimeoutException messages are typically null, and other timeout paths in this codebase (e.g., AbstractTaskExecutor) don't set a title for timeout errors, so this creates inconsistent error payloads.

                WorkflowError.timeout()
                    .instance(taskContext.position().jsonPointer())
                    .title(cause.getMessage())
                    .build(),

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:206

• This test only asserts that a WorkflowException occurred, but it doesn't assert that the failure is specifically the expected timeout (status 408). Strengthening the assertion makes the test validate the new attempt.duration behavior more precisely.

        .hasCauseInstanceOf(WorkflowException.class);

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:213

• handleTimeoutException sets the timeout error title from cause.getMessage(), but the TimeoutException created by CompletableFuture.orTimeout(...) typically has a null/empty message. This results in a non-informative title and is inconsistent with the existing task timeout handling in AbstractTaskExecutor (which does not set a title for timeouts).

            ? new WorkflowException(
                WorkflowError.timeout()
                    .instance(taskContext.position().jsonPointer())
                    .title(cause.getMessage())
                    .build(),

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:206

• testAttemptDuration() currently only asserts that a WorkflowException occurred, but it doesn’t verify that the failure is specifically the new per-attempt timeout (type/status) or that no retry happened (since the catch filter is for 404 communication errors). Strengthening the assertions makes the test reliably validate attempt.duration behavior.

    assertThatThrownBy(
            () ->
                app.workflowDefinition(
                        readWorkflowFromClasspath(
                            "workflows-samples/try-catch-retry-attempt-duration.yaml"))

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:216

• handleTimeoutException returns failedFuture(e) for non-timeout failures, which can preserve a CompletionException wrapper and make downstream error handling/reporting inconsistent. Unwrap to the underlying cause for the non-timeout path (and consider aligning the timeout WorkflowError building with AbstractTaskExecutor by not setting a potentially-null title from TimeoutException#getMessage()).

  private CompletableFuture<WorkflowModel> handleTimeoutException(
      Throwable e, TaskContext taskContext) {
    Throwable cause = e instanceof CompletionException ? e.getCause() : e;
    return CompletableFuture.failedFuture(
        cause instanceof TimeoutException
            ? new WorkflowException(
                WorkflowError.timeout()
                    .instance(taskContext.position().jsonPointer())
                    .title(cause.getMessage())
                    .build(),
                cause)
            : e);

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:189

• attempt.duration is currently enforced on the entire try/catch/retry chain because orTimeout() is applied after exceptionallyCompose(handleException). Since handleException() recursively calls doIt() for retries, the timeout covers the retry delay and subsequent attempts as well, which contradicts the intended per-attempt timeout behavior (a timeout on attempt #1 should still allow attempt #2 to run with its own timeout). Apply the timeout to the per-attempt execution future before routing failures into handleException() so timeouts become a WorkflowException(timeout) that can be matched by the catch/retry logic per attempt.

This issue also appears on line 205 of the same file.

  private CompletableFuture<WorkflowModel> doIt(
      WorkflowContext workflow, TaskContext taskContext, WorkflowModel model) {
    retryIntervalExecutor.ifPresent(r -> r.init(workflow, taskContext, model));
    CompletableFuture<WorkflowModel> future =
        TaskExecutorHelper.processTaskList(taskExecutor, workflow, Optional.of(taskContext), model)

impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java:233

• testAttemptDurationRetry currently expects the workflow to fail, but with a per-attempt timeout the first (delayed) response should time out, be caught as a timeout, and then the second (non-delayed) response should succeed. Also there is a stray standalone ; after the assertion. Update the test to assert successful completion and verify a retry occurred.

    assertThatThrownBy(
            () ->
                app.workflowDefinition(
                        readWorkflowFromClasspath(
                            "workflows-samples/try-catch-retry-attempt-duration-retry.yaml"))

[fjtirado]

onTimeout do not cancel the underlying future, it just complete it. Since onTimeout do not create a new completable, we need to copy the original one, keep its reference and cancel it.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java:218

• This branch fails the future with e, which can be a CompletionException wrapper; the previous flow unwrapped CompletionException and propagated the underlying cause. Returning cause here preserves the original exception type/stack and avoids extra wrapping.

      return CompletableFuture.failedFuture(e);