feat: Add replay phase to JournalFailureExceptions in typed #32557
Conversation
| case NonFatal(cause) => | ||
| onRecoveryFailure(cause, None) | ||
| case ReplayMessagesFailure(cause) => | ||
| onRecoveryFailure(cause, Some(response), "failure-from-journal") |
There was a problem hiding this comment.
If this threw previously, that would lead to a new call to onRecoveryFailure, but maybe it makes more sense that it doesn't.
There was a problem hiding this comment.
It shouldn't throw since we have:
setup.onSignal(state.state, RecoveryFailed(cause), catchAndLog = true)
but much other code there as well, which hopefully doesn't throw. Should we guard against that with an additional catch inside onRecoveryFailure?
It should always end with:
throw new JournalFailureException(msg, cause)
| } catch { | ||
| case ex: UnstashException[_] => | ||
| // let supervisor handle it, don't treat as recovery failure | ||
| throw ex |
There was a problem hiding this comment.
I think that it is correct that unstash exceptions can only ever happen in onRecoveryComplete.
| } catch { | ||
| case NonFatal(ex) => | ||
| state = state.copy(repr.sequenceNr) | ||
| onRecoveryFailure(ex, eventForErrorReporting.toOption, "replaying-event") |
There was a problem hiding this comment.
interesting, onRecoveryFailure will always end with:
throw new JournalFailureException(msg, cause)
and then we caught that again in the outer catch, and called onRecoveryFailure.
Doesn't make sense with that double handling, so this should be fine.
| case NonFatal(cause) => | ||
| onRecoveryFailure(cause, None) | ||
| case ReplayMessagesFailure(cause) => | ||
| onRecoveryFailure(cause, Some(response), "failure-from-journal") |
There was a problem hiding this comment.
It shouldn't throw since we have:
setup.onSignal(state.state, RecoveryFailed(cause), catchAndLog = true)
but much other code there as well, which hopefully doesn't throw. Should we guard against that with an additional catch inside onRecoveryFailure?
It should always end with:
throw new JournalFailureException(msg, cause)
onRecoveryFailurecan occur and log its message in three ways:The second one, in particular, is difficult to distinguish from the third (both will log the message without regard to a particular event), which is unfortunate since they're very different problems (the second is a problem in the recovery completed handler, the third is either infra (incl. journal plugin) or serialization).