Fix RetryingRpcClientTest.doNotRetryWhenResponseIsCancelled

[schiemon]

Motivation

RetryingRpcClientTest.doNotRetryWhenResponseIsCancelled is brittle because it assumes that cancelling the response after delegating to RetryingRpcClient will always result in exactly one attempt. However, there is no guarantee about the number of attempts made between res = delegate.execute() and res.cancel().

I propose rewriting the test to verify two guarantees:

1. If the response is cancelled before RetryingRpcClient.execute is called, RetryingRpcClient performs no attempts.
2. If the response is cancelled after RetryingRpcClient.execute is called, RetryingRpcClient performs at most one additional attempt after cancellation.

The first guarantee is straightforward to test. For the second, we currently lack a way to signal cancellation before invoking RetryingRpcClient.execute(ctx, req), since we do not yet have a handle to the response. Additionally, RetryingRpcClient currently ignores ctx.isCancelled() during retries.

To address this, I propose adding a ctx.isCancelled() check before each retry attempt. This allows the doNotRetryWhenResponseIsCancelled test to call ctx.cancel() before proceeding through the decorator chain, ensuring RetryingRpcClient observes the cancelled state and skips further retries.

See also this comment for additional context.

Modifications

• Added a ctx.isCancelled() check in RetryingRpcClient before scheduling each retry.
• Enhanced RetryingRpcClientTest.doNotRetryWhenResponseIsCancelled to cancel the request/response with varying delays.

Result

• RetryingRpcClientTest.doNotRetryWhenResponseIsCancelled is no longer flaky.
• RetryingRpcClient respects ctx.isCancelled

[github-actions]

🔍 Build Scan® (commit: f6cc34e)

Job name	Status	Build Scan®
build-ubicloud-standard-16-jdk-8	✅	https://ge.armeria.dev/s/zrkrwly4k2ruo
build-ubicloud-standard-16-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/6egqcrxkku5wu
build-ubicloud-standard-16-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/ocoepidoyl6su
build-ubicloud-standard-16-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/hbzepofea3erm
build-ubicloud-standard-16-jdk-17-leak	✅	https://ge.armeria.dev/s/qym57vbdqnh76
build-ubicloud-standard-16-jdk-11	✅	https://ge.armeria.dev/s/7usklwktho4pq
build-macos-latest-jdk-21	✅	https://ge.armeria.dev/s/ckkeg6q3ffdrm

[ikhoon]

Looks good!

[minwoox]

Thanks!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.09%. Comparing base (8150425) to head (f6cc34e).
⚠️ Report is 189 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6399      +/-   ##
============================================
- Coverage     74.46%   74.09%   -0.37%     
- Complexity    22234    22993     +759     
============================================
  Files          1963     2061      +98     
  Lines         82437    86125    +3688     
  Branches      10764    11310     +546     
============================================
+ Hits          61385    63813    +2428     
- Misses        15918    16898     +980     
- Partials       5134     5414     +280     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.