Rethrow undeclared exceptions

[ttulka]

Not sure if this is a "works as designed" problem, but it seems to be useful to support this use-case.
So far, I was not able to make it work:

I need the retry mechanism to be triggered only for declared exceptions and let others be rethrown.

With this code:

@Retryable(include = {ToRetryException.class}, recover = "recoverRetrieveFromExternal")
public String retrieveFromExternal(String arg) {
    return externalService.retrieve(arg);
}
@Recover
private String recoverRetrieveFromExternal(Throwable t, String arg) {
    return "Recover: " + arg;
}

When an exception different from ToRetryException is thrown by externalService.retrieve(arg) the recover fallback is used.

With stateful = true, all exceptions including ToRetryException are rethrown.

Basically, I need this test suite to pass:

@Test
void ok() {
    when(externalService.retrieve(anyString())).thenReturn("TEST");
    assertThat(client.retrieveFromExternal("")).isEqualTo("TEST");
}
@Test
void rethrows_the_exception() {
    when(externalService.retrieve(anyString())).thenThrow(new RuntimeException());
    assertThrows(RuntimeException.class, () -> client.retrieveFromExternal(""));
}
@Test
void retries_and_recovers() {
    when(externalService.retrieve(anyString())).thenThrow(new ToRetryException());
    assertThat(client.retrieveFromExternal("test")).isEqualTo("Recover: test");
}

Is there a way to do that? Thanks!

[garyrussell]

Don't use GitHub issues to ask questions; they are for bug reports or asking for new features; ask questions on Stack Overflow or use the Discussions tab.

Change the recover method so that it only handles ToRetryException, not all Throwables.

[garyrussell]

It generally looks ok but, unfortunately, this project is still based on Java6 so we can't add a new method to the RetryPolicy (can't use default in Java 6) because it is a breaking change - we will probably be moving to Java 17 in the 2.0 line. It would need to be a new interface (with an instanceof check).

Also, although not needed for your use case, for consistency, we should add a flag similar to throwLastExceptionOnExhausted to the RecoverAnnotationRecoveryHandler (e.g. throwLastExceptionWhenNoRecoverMethod) so it throws the raw exception instead of wrapping it in a ExhaustedRetryException when there is no matching @Recover method.

[ttulka]

I see. In that case I guess !((SimpleRetryPolicy) retryPolicy).retryForException(context.getLastThrowable()) would do the job for now as SimpleRetryPolicy is the default implementation in template anyway.

I have created a PR: #265

I will align RecoverAnnotationRecoveryHandler afterward.

[ttulka]

Hmm, implemented like that, I think no changes in RecoverAnnotationRecoveryHandler are necessary as recoveryCallback.recover(context) is not called at all when the rethrow flag is set.

[garyrussell]

Right; but what about a retryable exception that has no compatible @Recover method? - we need consistency - when rethrow is true it should rethrow the raw exception and not wrap it, for consistency with not-retryable exceptions.

[ttulka]

Implemented in the second commit of #265

[garyrussell]

Converting to a discussion.

[ttulka]

Sorry about that!