Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RetryContext#EXHAUSTED in RetryListener#onError #149

Open
icyerasor opened this issue Feb 28, 2019 · 9 comments
Open

RetryContext#EXHAUSTED in RetryListener#onError #149

icyerasor opened this issue Feb 28, 2019 · 9 comments

Comments

@icyerasor
Copy link

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.

@dsyer
Copy link
Member

dsyer commented Apr 4, 2019

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?

@icyerasor
Copy link
Author

icyerasor commented Apr 8, 2019

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

@fransflippo
Copy link

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.

@dsyer
Copy link
Member

dsyer commented Nov 11, 2019

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)?

@fransflippo
Copy link

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?

@dsyer
Copy link
Member

dsyer commented Nov 13, 2019

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.

@eloykramar
Copy link

eloykramar commented Jan 8, 2020

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.

@dsyer
Copy link
Member

dsyer commented Jan 9, 2020

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

@eloykramar
Copy link

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants