Skip to content

Commit fdcfb4d

Browse files
committed
Revert "GH-264: Add retrhro to @retryable"
This reverts commit ec245fd.
1 parent e1ebf23 commit fdcfb4d

File tree

9 files changed

+45
-179
lines changed

9 files changed

+45
-179
lines changed

Diff for: src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java

+5-10
Original file line numberDiff line numberDiff line change
@@ -217,16 +217,13 @@ private MethodInterceptor getStatelessInterceptor(Object target, Method method,
217217
RetryTemplate template = createTemplate(retryable.listeners());
218218
template.setRetryPolicy(getRetryPolicy(retryable));
219219
template.setBackOffPolicy(getBackoffPolicy(retryable.backoff()));
220-
template.setNoRecoveryForNotRetryable(retryable.rethrow());
221220
return RetryInterceptorBuilder.stateless().retryOperations(template).label(retryable.label())
222-
.recoverer(getRecoverer(target, method, retryable.rethrow())).build();
221+
.recoverer(getRecoverer(target, method)).build();
223222
}
224223

225224
private MethodInterceptor getStatefulInterceptor(Object target, Method method, Retryable retryable) {
226-
boolean rethrow = retryable.rethrow();
227225
RetryTemplate template = createTemplate(retryable.listeners());
228226
template.setRetryContextCache(this.retryContextCache);
229-
template.setNoRecoveryForNotRetryable(rethrow);
230227

231228
CircuitBreaker circuit = AnnotatedElementUtils.findMergedAnnotation(method, CircuitBreaker.class);
232229
if (circuit == null) {
@@ -244,15 +241,15 @@ private MethodInterceptor getStatefulInterceptor(Object target, Method method, R
244241
label = method.toGenericString();
245242
}
246243
return RetryInterceptorBuilder.circuitBreaker().keyGenerator(new FixedKeyGenerator("circuit"))
247-
.retryOperations(template).recoverer(getRecoverer(target, method, rethrow)).label(label).build();
244+
.retryOperations(template).recoverer(getRecoverer(target, method)).label(label).build();
248245
}
249246
RetryPolicy policy = getRetryPolicy(retryable);
250247
template.setRetryPolicy(policy);
251248
template.setBackOffPolicy(getBackoffPolicy(retryable.backoff()));
252249
String label = retryable.label();
253250
return RetryInterceptorBuilder.stateful().keyGenerator(this.methodArgumentsKeyGenerator)
254251
.newMethodArgumentsIdentifier(this.newMethodArgumentsIdentifier).retryOperations(template).label(label)
255-
.recoverer(getRecoverer(target, method, rethrow)).build();
252+
.recoverer(getRecoverer(target, method)).build();
256253
}
257254

258255
private long getOpenTimeout(CircuitBreaker circuit) {
@@ -296,7 +293,7 @@ private RetryListener[] getListenersBeans(String[] listenersBeanNames) {
296293
return listeners;
297294
}
298295

299-
private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method, boolean rethrow) {
296+
private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method) {
300297
if (target instanceof MethodInvocationRecoverer) {
301298
return (MethodInvocationRecoverer<?>) target;
302299
}
@@ -313,9 +310,7 @@ public void doWith(Method method) throws IllegalArgumentException, IllegalAccess
313310
if (!foundRecoverable.get()) {
314311
return null;
315312
}
316-
RecoverAnnotationRecoveryHandler recoveryHandler = new RecoverAnnotationRecoveryHandler<Object>(target, method);
317-
recoveryHandler.setThrowLastExceptionWhenNoRecoverMethod(rethrow);
318-
return recoveryHandler;
313+
return new RecoverAnnotationRecoveryHandler<Object>(target, method);
319314
}
320315

321316
private RetryPolicy getRetryPolicy(Annotation retryable) {

Diff for: src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2022 the original author or authors.
2+
* Copyright 2013-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -64,23 +64,16 @@ public class RecoverAnnotationRecoveryHandler<T> implements MethodInvocationReco
6464

6565
private String recoverMethodName;
6666

67-
private boolean throwLastExceptionWhenNoRecoverMethod;
68-
6967
public RecoverAnnotationRecoveryHandler(Object target, Method method) {
7068
this.target = target;
7169
init(target, method);
7270
}
7371

74-
public void setThrowLastExceptionWhenNoRecoverMethod(boolean throwLastExceptionWhenNoRecoverMethod) {
75-
this.throwLastExceptionWhenNoRecoverMethod = throwLastExceptionWhenNoRecoverMethod;
76-
}
77-
7872
@Override
7973
public T recover(Object[] args, Throwable cause) {
8074
Method method = findClosestMatch(args, cause.getClass());
8175
if (method == null) {
82-
throw throwLastExceptionWhenNoRecoverMethod && cause instanceof RuntimeException ? (RuntimeException) cause
83-
: new ExhaustedRetryException("Cannot locate recovery method", cause);
76+
throw new ExhaustedRetryException("Cannot locate recovery method", cause);
8477
}
8578
SimpleMetadata meta = this.methods.get(method);
8679
Object[] argsToUse = meta.getArgs(cause, args);

Diff for: src/main/java/org/springframework/retry/annotation/Retryable.java

+1-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2022 the original author or authors.
2+
* Copyright 2014-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -130,12 +130,4 @@
130130
*/
131131
String[] listeners() default {};
132132

133-
/**
134-
* When true raw exceptions are thrown without being wrapped and no recovery is
135-
* performed for not-retryable exceptions.
136-
* @return true if to rethrow raw exceptions, default false
137-
* @since 1.3.3
138-
*/
139-
boolean rethrow() default false;
140-
141133
}

Diff for: src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java

+10-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2006-2022 the original author or authors.
2+
* Copyright 2006-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -199,16 +199,6 @@ public RetryContext open(RetryContext parent) {
199199
return new SimpleRetryContext(parent);
200200
}
201201

202-
/**
203-
* Delegates to an exception classifier.
204-
* @param ex
205-
* @return true if this exception or its ancestors have been registered as retryable.
206-
* @since 1.3.3
207-
*/
208-
public boolean retryForException(Throwable ex) {
209-
return this.retryableClassifier.classify(ex);
210-
}
211-
212202
private static class SimpleRetryContext extends RetryContextSupport {
213203

214204
public SimpleRetryContext(RetryContext parent) {
@@ -217,6 +207,15 @@ public SimpleRetryContext(RetryContext parent) {
217207

218208
}
219209

210+
/**
211+
* Delegates to an exception classifier.
212+
* @param ex
213+
* @return true if this exception or its ancestors have been registered as retryable.
214+
*/
215+
private boolean retryForException(Throwable ex) {
216+
return this.retryableClassifier.classify(ex);
217+
}
218+
220219
@Override
221220
public String toString() {
222221
return ClassUtils.getShortName(getClass()) + "[maxAttempts=" + this.maxAttempts + "]";

Diff for: src/main/java/org/springframework/retry/support/RetryTemplate.java

+10-19
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2006-2022 the original author or authors.
2+
* Copyright 2006-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -96,8 +96,6 @@ public class RetryTemplate implements RetryOperations {
9696

9797
private boolean throwLastExceptionOnExhausted;
9898

99-
private boolean noRecoveryForNotRetryable;
100-
10199
/**
102100
* Main entry point to configure RetryTemplate using fluent API. See
103101
* {@link RetryTemplateBuilder} for usage examples and details.
@@ -126,14 +124,6 @@ public void setThrowLastExceptionOnExhausted(boolean throwLastExceptionOnExhaust
126124
this.throwLastExceptionOnExhausted = throwLastExceptionOnExhausted;
127125
}
128126

129-
/**
130-
* @param noRecoveryForNotRetryable the noRecoveryForNotRetryable to set
131-
* @since 1.3.3
132-
*/
133-
public void setNoRecoveryForNotRetryable(boolean noRecoveryForNotRetryable) {
134-
this.noRecoveryForNotRetryable = noRecoveryForNotRetryable;
135-
}
136-
137127
/**
138128
* Public setter for the {@link RetryContextCache}.
139129
* @param retryContextCache the {@link RetryContextCache} to set.
@@ -376,6 +366,7 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback
376366
}
377367
throw RetryTemplate.<E>wrapIfNecessary(e);
378368
}
369+
379370
}
380371

381372
/*
@@ -393,7 +384,7 @@ protected <T, E extends Throwable> T doExecute(RetryCallback<T, E> retryCallback
393384
}
394385

395386
exhausted = true;
396-
return handleRetryExhausted(recoveryCallback, context, state, retryPolicy);
387+
return handleRetryExhausted(recoveryCallback, context, state);
397388

398389
}
399390
catch (Throwable e) {
@@ -471,6 +462,7 @@ private void registerContext(RetryContext context, RetryState state) {
471462
* was encountered
472463
*/
473464
protected RetryContext open(RetryPolicy retryPolicy, RetryState state) {
465+
474466
if (state == null) {
475467
return doOpenInternal(retryPolicy);
476468
}
@@ -504,6 +496,7 @@ protected RetryContext open(RetryPolicy retryPolicy, RetryState state) {
504496
context.removeAttribute(RetryContext.EXHAUSTED);
505497
context.removeAttribute(RetryContext.RECOVERED);
506498
return context;
499+
507500
}
508501

509502
private RetryContext doOpenInternal(RetryPolicy retryPolicy, RetryState state) {
@@ -536,16 +529,12 @@ private RetryContext doOpenInternal(RetryPolicy retryPolicy) {
536529
* @return T the payload to return
537530
* @throws Throwable if there is an error
538531
*/
539-
protected <T> T handleRetryExhausted(RecoveryCallback<T> recoveryCallback, RetryContext context, RetryState state,
540-
RetryPolicy retryPolicy) throws Throwable {
532+
protected <T> T handleRetryExhausted(RecoveryCallback<T> recoveryCallback, RetryContext context, RetryState state)
533+
throws Throwable {
541534
context.setAttribute(RetryContext.EXHAUSTED, true);
542535
if (state != null && !context.hasAttribute(GLOBAL_STATE)) {
543536
this.retryContextCache.remove(state.getKey());
544537
}
545-
if (this.noRecoveryForNotRetryable && retryPolicy instanceof SimpleRetryPolicy
546-
&& !((SimpleRetryPolicy) retryPolicy).retryForException(context.getLastThrowable())) {
547-
throw context.getLastThrowable();
548-
}
549538
if (recoveryCallback != null) {
550539
T recovered = recoveryCallback.recover(context);
551540
context.setAttribute(RetryContext.RECOVERED, true);
@@ -559,7 +548,7 @@ protected <T> T handleRetryExhausted(RecoveryCallback<T> recoveryCallback, Retry
559548
}
560549

561550
protected <E extends Throwable> void rethrow(RetryContext context, String message) throws E {
562-
if (this.throwLastExceptionOnExhausted || this.noRecoveryForNotRetryable) {
551+
if (this.throwLastExceptionOnExhausted) {
563552
@SuppressWarnings("unchecked")
564553
E rethrow = (E) context.getLastThrowable();
565554
throw rethrow;
@@ -583,13 +572,15 @@ protected boolean shouldRethrow(RetryPolicy retryPolicy, RetryContext context, R
583572
}
584573

585574
private <T, E extends Throwable> boolean doOpenInterceptors(RetryCallback<T, E> callback, RetryContext context) {
575+
586576
boolean result = true;
587577

588578
for (RetryListener listener : this.listeners) {
589579
result = result && listener.open(context, callback);
590580
}
591581

592582
return result;
583+
593584
}
594585

595586
private <T, E extends Throwable> void doCloseInterceptors(RetryCallback<T, E> callback, RetryContext context,

Diff for: src/test/java/org/springframework/retry/annotation/EnableRetryTests.java

+1-39
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2022 the original author or authors.
2+
* Copyright 2014-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -255,22 +255,6 @@ public void testExpression() throws Exception {
255255
context.close();
256256
}
257257

258-
@Test
259-
public void rethrow() {
260-
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class);
261-
RethrowService service = context.getBean(RethrowService.class);
262-
for (int i = 0; i < 3; i++) {
263-
try {
264-
service.service();
265-
}
266-
catch (RuntimeException e) {
267-
assertEquals("Planned", e.getMessage());
268-
}
269-
}
270-
assertEquals(3, service.getCount());
271-
context.close();
272-
}
273-
274258
private Object target(Object target) {
275259
if (!AopUtils.isAopProxy(target)) {
276260
return target;
@@ -450,11 +434,6 @@ public ExcludesOnlyService excludesOnly() {
450434
return new ExcludesOnlyService();
451435
}
452436

453-
@Bean
454-
public RethrowService rethrowService() {
455-
return new RethrowService();
456-
}
457-
458437
@Bean
459438
public MethodInterceptor retryInterceptor() {
460439
return RetryInterceptorBuilder.stateless().maxAttempts(5).build();
@@ -717,23 +696,6 @@ public int getCount() {
717696

718697
}
719698

720-
private static class RethrowService {
721-
722-
private int count = 0;
723-
724-
@Retryable(include = IllegalArgumentException.class, rethrow = true)
725-
public void service() {
726-
if (this.count++ < 2) {
727-
throw new RuntimeException("Planned");
728-
}
729-
}
730-
731-
public int getCount() {
732-
return this.count;
733-
}
734-
735-
}
736-
737699
public static class ExceptionChecker {
738700

739701
public boolean shouldRetry(Throwable t) {

0 commit comments

Comments
 (0)