He-Pin opened a new issue, #3234:
URL: https://github.com/apache/pekko/issues/3234
### Motivation
When a `PersistentActor`'s `RecoveryCompleted` handler throws an exception,
the actor enters an infinite restart loop instead of stopping. All other
recovery failure paths (replayed events, snapshot loading, journal failures,
recovery timeout) explicitly stop the actor via `context.stop(self)` and log
the failure via `onRecoveryFailure`. The `RecoveryCompleted` path is the only
one that lacks this handling. The typed persistence API
(`pekko-persistence-typed`) correctly stops the actor in this scenario.
### Current behavior
In `Eventsourced.scala`, the `recovering` state handles `RecoverySuccess` as
follows (lines 786-789):
```scala
try Eventsourced.super.aroundReceive(recoveryBehavior, RecoveryCompleted)
finally transitToProcessingState() // in finally in case exception and
resume strategy
// if exception from RecoveryCompleted the permit is returned in below catch
returnRecoveryPermit()
```
When the `RecoveryCompleted` handler throws, the exception propagates to the
outer catch (lines 810-813):
```scala
catch {
case NonFatal(e) =>
returnRecoveryPermit()
throw e
}
```
This rethrows to the `ActorCell`, which applies the default supervisor
strategy (`Restart` for `Exception`). The actor restarts, re-enters recovery,
throws again, and loops indefinitely.
Compare with every other recovery failure path in the same state, e.g.
`ReplayedMessage` (lines 772-778):
```scala
catch {
case NonFatal(t) =>
timeoutCancellable.cancel()
try onRecoveryFailure(t, Some(p.payload))
finally context.stop(self)
returnRecoveryPermit()
}
```
All other paths call `onRecoveryFailure` and `context.stop(self)`. The
`RecoveryCompleted` path calls neither.
### Expected behavior
When the `RecoveryCompleted` handler throws, the actor should:
1. Call `onRecoveryFailure(cause, event = None)` to log the error
2. Call `context.stop(self)` to stop the actor
3. Call `returnRecoveryPermit()` to release the recovery permit
This matches the behavior of all other recovery failure paths in the same
file and the behavior of the typed persistence API (where the default
`supervisionStrategy` for `JournalFailureException` is
`SupervisorStrategy.stop`, see `EventSourcedBehaviorImpl.scala` line 120).
### Code evidence
-
`persistence/src/main/scala/org/apache/pekko/persistence/Eventsourced.scala:786-789`
-- the `RecoveryCompleted` handler invocation without `onRecoveryFailure` or
`context.stop(self)`
-
`persistence/src/main/scala/org/apache/pekko/persistence/Eventsourced.scala:810-813`
-- the outer catch that returns the permit and rethrows
-
`persistence/src/main/scala/org/apache/pekko/persistence/Eventsourced.scala:772-778`
-- the `ReplayedMessage` handler showing the correct pattern
-
`persistence/src/main/scala/org/apache/pekko/persistence/Eventsourced.scala:676-681`
-- the `SnapshotOffer` handler showing the correct pattern
-
`persistence/src/test/scala/org/apache/pekko/persistence/RecoveryPermitterSpec.scala:174-197`
-- existing test that documents the restart loop behavior (comment: `// it's
restarting`), verifying only that the permit is returned, not that the actor
lifecycle is correct
-
`persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/internal/EventSourcedBehaviorImpl.scala:120`
-- typed API default `SupervisorStrategy.stop` for journal failures
### Reproduction
1. Create a `PersistentActor` whose `receiveRecover` throws when it receives
`RecoveryCompleted`
2. Persist at least one event and restart the actor
3. Observe: the actor enters an infinite restart loop (repeated
`RecoveryCompleted` -> exception -> restart -> `RecoveryCompleted` -> exception
-> ...)
4. Expected: the actor should stop after the first failure, logging the
error via `onRecoveryFailure`
Suggested test: update `RecoveryPermitterSpec` "return permit when actor
throws from RecoveryCompleted" to assert the actor stops (is terminated) after
the first `RecoveryCompleted` exception, rather than expecting 5+ restarts.
### Impact
- **Module**: `pekko-persistence` (classic PersistentActor API)
- **Affected users**: Any user whose `receiveRecover` handler for
`RecoveryCompleted` can throw (e.g., state validation, external resource
initialization). A single exception causes an infinite restart loop, consuming
resources and potentially flooding logs.
- **Not affected**: `pekko-persistence-typed` (already handles this
correctly via `SupervisorStrategy.stop`)
--
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]
