[I] PersistentActor does not stop when RecoveryCompleted handler throws [pekko]

["He-Pin (via GitHub)" <gi...@apache.org>]

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: notifications-unsubscribe@pekko.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org

Re: [I] PersistentActor does not stop when RecoveryCompleted handler throws [pekko]

["He-Pin (via GitHub)" <gi...@apache.org>]

He-Pin closed issue #3234: PersistentActor does not stop when RecoveryCompleted handler throws
URL: https://github.com/apache/pekko/issues/3234


-- 
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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org