He-Pin opened a new issue, #3233:
URL: https://github.com/apache/pekko/issues/3233
### Motivation
`pekko.pattern.retry` invokes the `attempt` function `N+1` times when
`attempts=N` is specified. This off-by-one error means users get one more
side-effecting invocation than expected — an extra network call, database
query, or other operation that the `attempts` parameter was supposed to cap.
### Current behavior
In `RetrySupport.scala`, the private `retry` method (line 382–396) has an
`if/else` branch:
```scala
// RetrySupport.scala:382-396
if (maxAttempts - attempted > 0) {
val result = tryAttempt()
// ... on failure, recurse via doRetry(attempted + 1)
} else {
tryAttempt() // <-- executes after maxAttempts is exhausted
}
```
When `maxAttempts - attempted <= 0` (all attempts used), the `else` branch
still calls `tryAttempt()`, producing one extra invocation.
Trace with `attempts=5`, all failures:
| Recursive call | `attempted` | Guard `5 - attempted > 0` | `tryAttempt()`
invoked? |
|---|---|---|---|
| 1 | 0 | 5 > 0 — true | Yes (attempt #1) |
| 2 | 1 | 4 > 0 — true | Yes (attempt #2) |
| 3 | 2 | 3 > 0 — true | Yes (attempt #3) |
| 4 | 3 | 2 > 0 — true | Yes (attempt #4) |
| 5 | 4 | 1 > 0 — true | Yes (attempt #5) |
| 6 | 5 | 0 > 0 — **false** | **Yes (attempt #6 — extra)** |
The existing tests in `RetrySpec.scala` encode this behavior. For example,
the test "return a failure for a Future that would have succeeded but retries
were exhausted" (line 89) uses `attempts=5` and expects the error message `"6"`
— proving 6 invocations.
Edge case: `attempts=0` still calls `tryAttempt()` once, which contradicts
the meaning of "zero attempts."
### Expected behavior
`attempts=N` should invoke the attempt function exactly N times. When all
attempts are exhausted, the method should return the result of the last failed
attempt rather than executing one more time.
The parameter is named `attempts` (not `retries`) and is documented as
`@param attempts the maximum number of attempts`, which should mean N total
invocations.
### Code evidence
- `actor/src/main/scala/org/apache/pekko/pattern/RetrySupport.scala:394-396`
— the `else` branch that calls `tryAttempt()` after maxAttempts is exhausted
- `actor/src/main/scala/org/apache/pekko/pattern/RetrySupport.scala:382` —
the guard `if (maxAttempts - attempted > 0)` that allows exactly `maxAttempts`
entries into the if-branch, then falls through to the extra else-branch
invocation
-
`actor-tests/src/test/scala/org/apache/pekko/pattern/RetrySpec.scala:89-93` —
test expects `"6"` with `attempts=5`, confirming N+1 behavior
-
`actor-tests/src/test/scala/org/apache/pekko/pattern/RetrySpec.scala:128-133` —
test expects `"1000"` with `attempts=999`, confirming N+1 behavior
### Reproduction
```scala
import java.util.concurrent.atomic.AtomicInteger
import scala.concurrent.{Await, Future}
import scala.concurrent.duration._
val counter = new AtomicInteger(0)
val result = retry(
attempt = () => { counter.incrementAndGet(); Future.failed(new
RuntimeException("fail")) },
attempts = 3,
delay = 1.milli
)(system.dispatcher, system.scheduler)
Await.ready(result, 5.seconds)
assert(counter.get() == 3) // FAILS: actual value is 4
```
### Impact
- Module: `pekko-actor`, package `org.apache.pekko.pattern`
- Affects all overloads of `retry` (with/without delay, with/without
`shouldRetry` predicate, with/without exponential backoff), since they all
delegate to the same private `retry` method
- Users who rely on `attempts` to limit side effects (rate-limited APIs,
billing-per-request services, idempotency boundaries) will see one more
invocation than allowed
- **Note on binary compatibility**: fixing this will require updating the
existing tests in `RetrySpec.scala` that encode the N+1 behavior
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
