Allow for RecoveryCallback to throw original exception

[shollander]

The current design of the RetryTemplate and RecoveryCallback is to not throw any exception if a recovery callback is provided. There are cases where even after recovery, the caller wants to know if the original action failed or succeeded. This can be accomplished by providing a way for the recovery callback to throw the original exception. It is currently a little clunky to do so since RecoveryCallback has a signature of throws Exception, while retryContext.getLastThrowable() provides a Throwable, which is not compatible.

[dsyer]

I guess we could change the signature if we are going to bump to a new major version. Would that solve your problem? Under what circumstances would you care? Why is Exception not broad enough?

[shollander]

Because this will not compile:

RecoveryCallback<T> = retryContext -> {
    throw retryContext.getLastThrowable();
};

[dsyer]

Fair enough. But it's easy to work around isn't it? I guess that's what you meant by "clunky".

[shollander]

Exactly. It would require a lot of type checking and casting, etc.

RecoveryCallback<T> = retryContext -> {
    Throwable t = retryContext.getLastThrowable();
    if (t instanceof Exception) {
        throw (Exception)t;
    } else if (t instanceof Throwable) {
        throw new RuntimeException(t);
    }
};

So basically it requires six lines instead of one, and also requires wrapping the exception in the case of Throwable, which can make it more difficult to deal with later on.

[dsyer]

Sure, but you would put those 6 lines in a convenience class and only use one line in practice, if this pattern was common. We can put such a convenience class in spring-retry if you like, but there are plenty already out there. Plus it’s easy to write your own.

[shollander]

Granted, although that doesn't address the second issue of unnecessarily wrapping the original exception in certain cases.

[dsyer]

Do you have an actual use case where that matters?

[shollander]

Not at the moment. My point was just that the API is slightly inconsistent here and can cause inconveniences. If you don't want to fix it there are workarounds.