Skip to content

Preserve unset fields when merging retry options - #3139

Open
Anthounn wants to merge 1 commit into
temporalio:mainfrom
Anthounn:fix/merge-partial-retry-options
Open

Anthounn wants to merge 1 commit into
temporalio:mainfrom
Anthounn:fix/merge-partial-retry-options

Conversation

@Anthounn

@Anthounn Anthounn commented Oct 9, 2026

Copy link
Copy Markdown

What changed?

  • Skip the initial-interval and backoff-coefficient setters when their merged values are unset.
  • Add regression coverage for empty and partial policies, override precedence, deferred defaults, and subsequent @MethodRetry merging.
  • Add a changelog entry and clarify that merging does not apply defaults.

Why?

RetryOptions.Builder.build() permits partial policies, but RetryOptions.merge(RetryOptions) unconditionally passed their unset values to setters that reject them. For example:

RetryOptions.newBuilder().setMaximumAttempts(3).build()
    .merge(RetryOptions.newBuilder().setDoNotRetry("PermanentFailure").build());

This throws NullPointerException for the unset initial interval. When an initial interval is supplied but neither policy sets a coefficient, merging instead throws IllegalArgumentException for coefficient 0.0.

Preserving the unset values allows callers to compose retry policies before applying defaults or annotation values, without changing precedence for explicitly configured fields.

Validation

  • Before the fix: 4 failures and 3 passes in RetryOptionsTest, reproducing both exceptions.
  • After the fix: all 8 RetryOptionsTest tests pass, including the 6 new tests.
  • ./gradlew --offline :temporal-sdk:spotlessApply and git diff --check pass.
  • Broader run: 143 passed, 2 skipped, no failures/errors across 20 suites (io.temporal.common.*, ActivityOptionsTest, and RetryOptionsUtilsTest).
    Two existing ActivityClientCallsInterceptorChainTest tests were skipped.
  • Windows, Java 21; full multi-module suite, external Temporal server, and Cloud tests not run.

Breaking changes?

None. No API signatures change; existing explicit-value precedence and null-override behavior are preserved.

Server PR

Not required.

@Anthounn
Anthounn requested a review from a team as a code owner October 9, 2026 17:04
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants