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

IllegalArgumentException when migrating from 1.3.1 to 1.3.2 and using class annotations #299

Closed
deadpassive opened this issue May 18, 2022 · 3 comments · Fixed by #300
Closed
Assignees
Labels
Milestone

Comments

@deadpassive
Copy link

Hi,

I am using a setup similar to the following, where I have a @Retryable defined on the class and also a @Recover method.

@Retryable(value = {MyFirstException.class, MySecondException.class})
@Service
public class MyService {

  public void myMethod(String myArg) {
    System.out.println("Doing a thing that will fail with args " + myArg);
    throw new MyFirstException();
  }

  @Recover
  public void recoverMyMethod(Exception e, String myArg) throws Exception {
    System.out.println("In recovery, with args " + myArg);
    throw e;
  }
}

When running with spring-retry v1.3.1 this worked fine and myMethod is run 3 times before recoverMyMethod is called a single time before the exception is finally thrown.

When running with v1.3.2 or v1.3.3 of spring-retry the behaviour changes and it also retries the recover method too. So myMethod is called 3 times and then recoverMyMethod is called 3 times before finally an IllegalArgumentException is thrown.

I think this exception is thrown because it is trying to call recoverMyMethod to recover from itself and gets confused when creating the args to call it and ends up trying to call it with (Exception, Exception) instead of (Exception, String).

Is this a bug in spring-retry, or are we using it incorrectly?

Here is an example project which illustrates the issue:
spring-retry-test.zip

@garyrussell
Copy link
Contributor

I am not sure (yet) what caused it, but the behavior has definitely changed; as a work-around, moving the @Retryable to the method (instead of declaring it at the class level) works.

@garyrussell garyrussell self-assigned this May 18, 2022
@garyrussell
Copy link
Contributor

It was broken by the fix for this #244

@garyrussell
Copy link
Contributor

garyrussell commented May 18, 2022

Another work around is to remove the public modifier from the recover method (or make it private); that prevents the interceptor from proxying that method.

garyrussell added a commit to garyrussell/spring-retry that referenced this issue May 18, 2022
Resolves spring-projects#299

Caused by the fix for spring-projects#244, recover methods should not be considered
as retryable, with class level annotation.

**cherry-pick to 1.3.x**
artembilan pushed a commit that referenced this issue May 18, 2022
Resolves #299

Caused by the fix for #244, recover methods should not be considered
as retryable, with class level annotation.

**cherry-pick to 1.3.x**
artembilan pushed a commit that referenced this issue May 18, 2022
Resolves #299

Caused by the fix for #244, recover methods should not be considered
as retryable, with class level annotation.

**cherry-pick to 1.3.x**

* Fix conflicts after cherry-picking: JUnit 4, proper `MethodCallback` import
@garyrussell garyrussell added this to the 2.0.0-M1 milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants