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

maxAttemptsExpression is not evaluated when an exceptionExpression is set #383

Closed
shubhgup11 opened this issue Sep 29, 2023 · 2 comments · Fixed by #384
Closed

maxAttemptsExpression is not evaluated when an exceptionExpression is set #383

shubhgup11 opened this issue Sep 29, 2023 · 2 comments · Fixed by #384
Assignees
Labels
Milestone

Comments

@shubhgup11
Copy link

@retryable doesnt seem to evaluate maxAttemptsExpression while creating the retryTemplate, if it has an exceptionExpression

This piece of code in AnnotationAwareRetryOperationsInterceptor.java should be modified i guess .

if (simple == null) {
			if (hasExpression) {
				simple = new ExpressionRetryPolicy(maxAttempts, policyMap, true, resolve(exceptionExpression),
						retryNotExcluded)
					.withBeanFactory(this.beanFactory);
			}
			else {
				simple = new SimpleRetryPolicy(maxAttempts, policyMap, true, retryNotExcluded);
				if (expression != null) {
					simple.maxAttemptsSupplier(() -> evaluate(expression, Integer.class, stateless));
				}
			}
		}

The maxAttempts supplies should be evaluated no matter , exceptionExpression is present or not.

This should be like this maybe

if (simple == null) {
			if (hasExpression) {
				simple = new ExpressionRetryPolicy(maxAttempts, policyMap, true, resolve(exceptionExpression),
						retryNotExcluded)
					.withBeanFactory(this.beanFactory);
			}
			else {
				simple = new SimpleRetryPolicy(maxAttempts, policyMap, true, retryNotExcluded);
			}
if (expression != null) {
					simple.maxAttemptsSupplier(() -> evaluate(expression, Integer.class, stateless));
				}
		}
@gaofengIt
Copy link

gaofengIt commented Sep 29, 2023 via email

@garyrussell
Copy link
Contributor

Confirmed - however, it only applies if retryFor (include) or noRetryFor (exclude) is provided because the expression is injected here:

if (includes.length == 0 && excludes.length == 0) {
simple = hasExpression
? new ExpressionRetryPolicy(resolve(exceptionExpression)).withBeanFactory(this.beanFactory)
: new SimpleRetryPolicy();
if (expression != null) {
simple.maxAttemptsSupplier(() -> evaluate(expression, Integer.class, stateless));
}
else {
simple.setMaxAttempts(maxAttempts);
}
}

@garyrussell garyrussell self-assigned this Oct 2, 2023
@garyrussell garyrussell added the bug label Oct 2, 2023
@garyrussell garyrussell added this to the 2.0.4 milestone Oct 2, 2023
garyrussell added a commit to garyrussell/spring-retry that referenced this issue Oct 2, 2023
Resolves spring-projects#383

If explicit exceptions are referenced, and an `exceptionExpression` is
provided, the `maxAttemptsExpression` was ignored.
artembilan pushed a commit that referenced this issue Oct 2, 2023
Resolves #383

If explicit exceptions are referenced, and an `exceptionExpression` is
provided, the `maxAttemptsExpression` was ignored.
@snicoll snicoll changed the title maxAttemptsExpression is never evaluated if you pass in exceptionExpression !! maxAttemptsExpression is not evaluated when an exceptionExpression is set Oct 3, 2023
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.

3 participants