Axonframework: Allow IOException through RetryingCallback

[Sam-Kruglov]

Inside RetryingCallback there this piece of code:

// We fail immediately when the exception is checked,
// or when it is a Deadlock Exception and we have an active unit of work.
if (!(cause instanceof RuntimeException)
        || (isCausedBy(cause, DeadlockException.class) && CurrentUnitOfWork.isStarted())
        || !retryScheduler.scheduleRetry(commandMessage, (RuntimeException) cause,
                                         new ArrayList<>(history),
                                         new RetryDispatch(commandMessage))) {
    delegate.onResult(commandMessage, commandResultMessage);
}

It will not allow to retry IOException which really is the best scenario for a that. Is there any idea behind it?

[smcvb]

Can you provide some reasoning why retrying on the IOException "really is the best scenario"? On face value, I am not convinced this should be changed around at the moment, but maybe your argument is valid. Or, your scenario has a cleaner solution then allowing IOExceptions in the RetryingCallback.

[Sam-Kruglov]

A command handler may use I/O operations which may fail. I see this as a perfect scenario for a retry, because that resource is most likely to be temporarily unavailable. For example, a service is down, so connection is refused, a file storage service is overloaded etc.

Also, if the service, which is down, contains the command handler, then we get a no handler found error, which is also perfectly valid for a retry if we are sure that handler actually exists.

[smcvb]

As of 4.2, the NoHandlerForCommandException has been made transient, thus setting it up for retry through the RetryScheduler. So that route should be covered already.

A part from that, I feel your I/O operations point and it seems reasonable to me. I am however still hesitant to set this in stone at this point in time. I would rather you have override the RetrySchedulers isExplicitlyNonTransient method to allow the IOException in your own application for now instead of enforcing this for everybody.

I'll ensure we'll discuss this with the team though, don't worry.
For now I assume the override suggestion of the isExplicitlyNonTransient solves your problem at hand and as such I will be closing this issue.

[Sam-Kruglov]

It does solve it, thanks!
Btw, I think there is 4.2 already but it does not seem to contain any commits

[smcvb]

Great to hear this resolves it for now @Sam-Kruglov!
4.2 has indeed been released, about 10 days ago now.

Expect some marketing info on this part soon.
For now though, you can just update your project to 4.2 if you want.

[Sam-Kruglov]

Thanks! Also, saw that this callback is called directly (calling the constructor) inside abstract command gateway, so I can't inherit that class simply, I would also have to inherit the command gateway to override that or just pass my callback explicitly. This makes it a bit less clean but still doable :)

[smcvb]

I meant overriding the RetyrScheduler i.o. the RetryCallback. This callback is only created when a RetyrScheduler and the check you're referencing is an 'or' between RuntimeException, a cause of DeadlockException and active unit of work or RetryScheduler#scheduleRetry call. Thus, adjusting the latter should do the trick for you.

[Sam-Kruglov]

I think you misunderstood.

// We fail immediately when the exception is checked,
// or when it is a Deadlock Exception and we have an active unit of work.
if (!(cause instanceof RuntimeException)
        || (isCausedBy(cause, DeadlockException.class) && CurrentUnitOfWork.isStarted())
        || !retryScheduler.scheduleRetry(commandMessage, (RuntimeException) cause,
                                         new ArrayList<>(history),
                                         new RetryDispatch(commandMessage))) {
    delegate.onResult(commandMessage, commandResultMessage);
}

So, if cause is not instance of RuntimeException then no other conditions would be even considered, because they are chained with OR. Any true in an OR chain is enough to give an answer, so my retryScheduler will not be called.

Also, I tried to override org.axonframework.commandhandling.gateway.AbstractCommandGateway#send but retryScheduler is declared private which makes it even uglier to implement.

I would suggest creating a default boolean isRetryable(Throwable throwable) inside RetryScheduler interface where you can define all the conditions including the deadlock and non transient stuff.

For now, my workaround is to catch an IOException and wrap it into java.io.UncheckedIOException

[smcvb]

That's indeed true, sorry for the misguidance there.
Wrapping the exception yourself is indeed the best approach for now, as we're not going to change this due to unexpected behaviour for other users.

Additionally, I did not mean to override the CommandGateway#send method, but the AbstractRetryScheduler#isExplicitlyNonTransient(Throwable) method.
Granted, I said RetryScheduler#scheduleRetry, instead of AbstractRetryScheduler#isExplicitlyNonTransient(Throwable), so I understand the misconception here.

We'll have a look at your suggestion, but as said for now, we're not putting to much effort in to this due to prioritization. I hope you understand.