spring-retry icon indicating copy to clipboard operation
spring-retry copied to clipboard

RetryContext#EXHAUSTED in RetryListener#onError

Open icyerasor opened this issue 6 years ago • 9 comments

In my RetryListener Implementation I would like to be able to know if the failed retry is the very last one / final one.

I thought that would be possible by checking whether the context is exhausted like this:

  @Bean
  public RetryListener retryListener() {
    return new RetryListenerSupport() {
      @Override
      public <T, E extends Throwable> void onError(RetryContext retryContext, RetryCallback<T, E> retryCallback, Throwable throwable) {
        if(retryContext.hasAttribute(RetryContext.EXHAUSTED)) {
          // last retry that will be performed, the exception will now be thrown to the caller
        } else {
          // there are more retries - exception suppressed: log the retry / count
        }
      }
    };
  }

turns out the retryContext never has a set EXHAUSTED-Attribute. In onClose it is set - but not useful to me. I did not find another (elegant) way to check whether the current retry retryContext.getRetryCount() is the final retry before the exception gets thrown to the caller.

Background: I'd like to log retries that have been performed - but not the final one, as that one will lead to an exception log anyway.

icyerasor avatar Feb 28 '19 12:02 icyerasor

You can ask the RetryPolicy if it canRetry(retryContext). That might not be super convenient, but it would tell you what you need to know, I think?

dsyer avatar Apr 04 '19 14:04 dsyer

Hmmn yes that could work - not sure how i can get a handle on a (or more exactly the matching) RetryPolicy in the onError-Scope though. PS: I'm using annotation-based @Retryable-Configuration

icyerasor avatar Apr 08 '19 12:04 icyerasor

Same issue here. Since the RetryListener is shared across multiple retry policies, there doesn't seem to be a way for it to get a handle on the "current" RetryPolicy. I'd like to log when a retry is done so it's obvious from reading the logs why the same call is repeated several times (and in the case of a backoff delay, why the system seems to be "hanging" for the duration of the backoff delay).

Why does RetryContext.exhaustedOnly not return the correct value in onError? Is onError called before setExhaustedOnly? In that case, would it be possible to add another hook, e.g. afterError that gets called after the error has been handled (and setExhaustedOnly called if no retries will be done)? Alternatively, can the RetryPolicy be passed into the handler methods on the RetryListener (I understand that would be a breaking change)?

Happy to submit a PR, just need to understand the reasoning behind how it currently works.

Thanks.

fransflippo avatar Nov 11 '19 07:11 fransflippo

I guess we could add the RetryPolicy to the RetryContext. But why can't you use the onClose() callback (OP said it "was not useful" but didn't say why)?

dsyer avatar Nov 11 '19 08:11 dsyer

According to the Javadoc, close() is only called after the last attempt (whether successful or ultimately failed). So it would not suffice for printing a "Xxx failed, will retry in x ms" type message. That would only be possible in onError(), as far as I can tell?

fransflippo avatar Nov 11 '19 19:11 fransflippo

Correct, I think (at least for stateless retry). So you need both callbacks (onError() and close()). The only problem is that you don't know for sure if there will actually be a retry attempt in onError()?

You could override the BackOffPolicy (which is only ever called if the error is going to be retried). Would that be a decent workaround? Beyond that I think adding a new attribute to the context would make most sense (e.g. RetryContext.CAN_RETRY), and then you could look at that in your interceptor.

dsyer avatar Nov 13 '19 11:11 dsyer

Hello! I have a similar issue, I want track errors on retries, but avoid the last because I throw particular ex on recovery and tracking differently.. using OnError last error is tracked twice.

I could override the BackOffPolicy, but in this scope not have the exception.

any update of this or another workaround that can I try?

Thanks.

eloykramar avatar Jan 08 '20 14:01 eloykramar

The last exception is always available from the RetryContext (via RetrySynchronizationManager).

dsyer avatar Jan 09 '20 13:01 dsyer

Oh sorry, I don't see that, It is very useful for me.

Thanks!

eloykramar avatar Jan 09 '20 14:01 eloykramar