diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index d2b46e7d..6c7815a0 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2018 the original author or authors. + * Copyright 2014-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -220,13 +220,16 @@ private MethodInterceptor getStatelessInterceptor(Object target, Method method, RetryTemplate template = createTemplate(retryable.listeners()); template.setRetryPolicy(getRetryPolicy(retryable)); template.setBackOffPolicy(getBackoffPolicy(retryable.backoff())); + template.setNoRecoveryForNotRetryable(retryable.rethrow()); return RetryInterceptorBuilder.stateless().retryOperations(template).label(retryable.label()) - .recoverer(getRecoverer(target, method)).build(); + .recoverer(getRecoverer(target, method, retryable.rethrow())).build(); } private MethodInterceptor getStatefulInterceptor(Object target, Method method, Retryable retryable) { + boolean rethrow = retryable.rethrow(); RetryTemplate template = createTemplate(retryable.listeners()); template.setRetryContextCache(this.retryContextCache); + template.setNoRecoveryForNotRetryable(rethrow); CircuitBreaker circuit = AnnotatedElementUtils.findMergedAnnotation(method, CircuitBreaker.class); if (circuit == null) { @@ -244,7 +247,7 @@ private MethodInterceptor getStatefulInterceptor(Object target, Method method, R label = method.toGenericString(); } return RetryInterceptorBuilder.circuitBreaker().keyGenerator(new FixedKeyGenerator("circuit")) - .retryOperations(template).recoverer(getRecoverer(target, method)).label(label).build(); + .retryOperations(template).recoverer(getRecoverer(target, method, rethrow)).label(label).build(); } RetryPolicy policy = getRetryPolicy(retryable); template.setRetryPolicy(policy); @@ -252,7 +255,7 @@ private MethodInterceptor getStatefulInterceptor(Object target, Method method, R String label = retryable.label(); return RetryInterceptorBuilder.stateful().keyGenerator(this.methodArgumentsKeyGenerator) .newMethodArgumentsIdentifier(this.newMethodArgumentsIdentifier).retryOperations(template).label(label) - .recoverer(getRecoverer(target, method)).build(); + .recoverer(getRecoverer(target, method, rethrow)).build(); } private long getOpenTimeout(CircuitBreaker circuit) { @@ -296,7 +299,7 @@ private RetryListener[] getListenersBeans(String[] listenersBeanNames) { return listeners; } - private MethodInvocationRecoverer getRecoverer(Object target, Method method) { + private MethodInvocationRecoverer getRecoverer(Object target, Method method, boolean rethrow) { if (target instanceof MethodInvocationRecoverer) { return (MethodInvocationRecoverer) target; } @@ -313,7 +316,9 @@ public void doWith(Method method) throws IllegalArgumentException, IllegalAccess if (!foundRecoverable.get()) { return null; } - return new RecoverAnnotationRecoveryHandler(target, method); + RecoverAnnotationRecoveryHandler recoveryHandler = new RecoverAnnotationRecoveryHandler(target, method); + recoveryHandler.setThrowLastExceptionWhenNoRecoverMethod(rethrow); + return recoveryHandler; } private RetryPolicy getRetryPolicy(Annotation retryable) { diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index c0511e4d..2cad5bc7 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -62,16 +62,23 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco private String recoverMethodName; + private boolean throwLastExceptionWhenNoRecoverMethod; + public RecoverAnnotationRecoveryHandler(Object target, Method method) { this.target = target; init(target, method); } + public void setThrowLastExceptionWhenNoRecoverMethod(boolean throwLastExceptionWhenNoRecoverMethod) { + this.throwLastExceptionWhenNoRecoverMethod = throwLastExceptionWhenNoRecoverMethod; + } + @Override public T recover(Object[] args, Throwable cause) { Method method = findClosestMatch(args, cause.getClass()); if (method == null) { - throw new ExhaustedRetryException("Cannot locate recovery method", cause); + throw throwLastExceptionWhenNoRecoverMethod && cause instanceof RuntimeException ? (RuntimeException) cause + : new ExhaustedRetryException("Cannot locate recovery method", cause); } SimpleMetadata meta = this.methods.get(method); Object[] argsToUse = meta.getArgs(cause, args); diff --git a/src/main/java/org/springframework/retry/annotation/Retryable.java b/src/main/java/org/springframework/retry/annotation/Retryable.java index 62c1d8f1..543ea82a 100644 --- a/src/main/java/org/springframework/retry/annotation/Retryable.java +++ b/src/main/java/org/springframework/retry/annotation/Retryable.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -130,4 +130,12 @@ */ String[] listeners() default {}; + /** + * When true raw exceptions are thrown without being wrapped and no recovery is + * performed for not-retryable exceptions. + * @return true if to rethrow raw exceptions, default false + * @since 1.3.3 + */ + boolean rethrow() default false; + } diff --git a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java index 9515d51f..f3a7828a 100644 --- a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2019 the original author or authors. + * Copyright 2006-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -199,23 +199,24 @@ public RetryContext open(RetryContext parent) { return new SimpleRetryContext(parent); } - private static class SimpleRetryContext extends RetryContextSupport { - - public SimpleRetryContext(RetryContext parent) { - super(parent); - } - - } - /** * Delegates to an exception classifier. * @param ex * @return true if this exception or its ancestors have been registered as retryable. + * @since 1.3.3 */ - private boolean retryForException(Throwable ex) { + public boolean retryForException(Throwable ex) { return this.retryableClassifier.classify(ex); } + private static class SimpleRetryContext extends RetryContextSupport { + + public SimpleRetryContext(RetryContext parent) { + super(parent); + } + + } + @Override public String toString() { return ClassUtils.getShortName(getClass()) + "[maxAttempts=" + this.maxAttempts + "]"; diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index 8bf4bf49..07262217 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2020 the original author or authors. + * Copyright 2006-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -96,6 +96,8 @@ public class RetryTemplate implements RetryOperations { private boolean throwLastExceptionOnExhausted; + private boolean noRecoveryForNotRetryable; + /** * Main entry point to configure RetryTemplate using fluent API. See * {@link RetryTemplateBuilder} for usage examples and details. @@ -124,6 +126,14 @@ public void setThrowLastExceptionOnExhausted(boolean throwLastExceptionOnExhaust this.throwLastExceptionOnExhausted = throwLastExceptionOnExhausted; } + /** + * @param noRecoveryForNotRetryable the noRecoveryForNotRetryable to set + * @since 1.3.3 + */ + public void setNoRecoveryForNotRetryable(boolean noRecoveryForNotRetryable) { + this.noRecoveryForNotRetryable = noRecoveryForNotRetryable; + } + /** * Public setter for the {@link RetryContextCache}. * @param retryContextCache the {@link RetryContextCache} to set. @@ -366,7 +376,6 @@ protected T doExecute(RetryCallback retryCallback } throw RetryTemplate.wrapIfNecessary(e); } - } /* @@ -384,7 +393,7 @@ protected T doExecute(RetryCallback retryCallback } exhausted = true; - return handleRetryExhausted(recoveryCallback, context, state); + return handleRetryExhausted(recoveryCallback, context, state, retryPolicy); } catch (Throwable e) { @@ -462,7 +471,6 @@ private void registerContext(RetryContext context, RetryState state) { * was encountered */ protected RetryContext open(RetryPolicy retryPolicy, RetryState state) { - if (state == null) { return doOpenInternal(retryPolicy); } @@ -496,7 +504,6 @@ protected RetryContext open(RetryPolicy retryPolicy, RetryState state) { context.removeAttribute(RetryContext.EXHAUSTED); context.removeAttribute(RetryContext.RECOVERED); return context; - } private RetryContext doOpenInternal(RetryPolicy retryPolicy, RetryState state) { @@ -529,12 +536,16 @@ private RetryContext doOpenInternal(RetryPolicy retryPolicy) { * @return T the payload to return * @throws Throwable if there is an error */ - protected T handleRetryExhausted(RecoveryCallback recoveryCallback, RetryContext context, RetryState state) - throws Throwable { + protected T handleRetryExhausted(RecoveryCallback recoveryCallback, RetryContext context, RetryState state, + RetryPolicy retryPolicy) throws Throwable { context.setAttribute(RetryContext.EXHAUSTED, true); if (state != null && !context.hasAttribute(GLOBAL_STATE)) { this.retryContextCache.remove(state.getKey()); } + if (this.noRecoveryForNotRetryable && retryPolicy instanceof SimpleRetryPolicy + && !((SimpleRetryPolicy) retryPolicy).retryForException(context.getLastThrowable())) { + throw context.getLastThrowable(); + } if (recoveryCallback != null) { T recovered = recoveryCallback.recover(context); context.setAttribute(RetryContext.RECOVERED, true); @@ -548,7 +559,7 @@ protected T handleRetryExhausted(RecoveryCallback recoveryCallback, Retry } protected void rethrow(RetryContext context, String message) throws E { - if (this.throwLastExceptionOnExhausted) { + if (this.throwLastExceptionOnExhausted || this.noRecoveryForNotRetryable) { @SuppressWarnings("unchecked") E rethrow = (E) context.getLastThrowable(); throw rethrow; @@ -572,7 +583,6 @@ protected boolean shouldRethrow(RetryPolicy retryPolicy, RetryContext context, R } private boolean doOpenInterceptors(RetryCallback callback, RetryContext context) { - boolean result = true; for (RetryListener listener : this.listeners) { @@ -580,7 +590,6 @@ private boolean doOpenInterceptors(RetryCallback } return result; - } private void doCloseInterceptors(RetryCallback callback, RetryContext context, diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index 7bff6aeb..b427bb5e 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2021 the original author or authors. + * Copyright 2014-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -255,6 +255,22 @@ public void testExpression() throws Exception { context.close(); } + @Test + public void rethrow() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class); + RethrowService service = context.getBean(RethrowService.class); + for (int i = 0; i < 3; i++) { + try { + service.service(); + } + catch (RuntimeException e) { + assertEquals("Planned", e.getMessage()); + } + } + assertEquals(3, service.getCount()); + context.close(); + } + private Object target(Object target) { if (!AopUtils.isAopProxy(target)) { return target; @@ -434,6 +450,11 @@ public ExcludesOnlyService excludesOnly() { return new ExcludesOnlyService(); } + @Bean + public RethrowService rethrowService() { + return new RethrowService(); + } + @Bean public MethodInterceptor retryInterceptor() { return RetryInterceptorBuilder.stateless().maxAttempts(5).build(); @@ -696,6 +717,23 @@ public int getCount() { } + private static class RethrowService { + + private int count = 0; + + @Retryable(include = IllegalArgumentException.class, rethrow = true) + public void service() { + if (this.count++ < 2) { + throw new RuntimeException("Planned"); + } + } + + public int getCount() { + return this.count; + } + + } + public static class ExceptionChecker { public boolean shouldRetry(Throwable t) { diff --git a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java index 82d2d623..ab4baf42 100644 --- a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java +++ b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -74,7 +74,16 @@ public void noMatch() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class)); this.expected.expect(ExhaustedRetryException.class); - handler.recover(new Object[] { "Dave" }, new Error("Planned")); + handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned")); + } + + @Test + public void noMatchWithRethrow() { + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( + new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class)); + handler.setThrowLastExceptionWhenNoRecoverMethod(true); + this.expected.expect(IllegalArgumentException.class); + handler.recover(new Object[] { "Dave" }, new IllegalArgumentException("Planned")); } @Test @@ -90,7 +99,6 @@ public void inAccessibleRecoverMethods() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new InAccessibleRecover(), foo); assertEquals(1, handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); - } @Test @@ -114,7 +122,6 @@ public void parentReturnTypeRecoverMethod() { @Test public void genericReturnStringValueTypeParentThrowableRecoverMethod() { - RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); @@ -128,7 +135,6 @@ public void genericReturnStringValueTypeParentThrowableRecoverMethod() { @Test public void genericReturnStringValueTypeChildThrowableRecoverMethod() { - RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); @@ -142,7 +148,6 @@ public void genericReturnStringValueTypeChildThrowableRecoverMethod() { @Test public void genericReturnOneValueTypeRecoverMethod() { - RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "bar", String.class)); @@ -209,7 +214,6 @@ public void genericNestedMapNumberStringReturnTypeRecoverMethod() { Map>> recoverResponseMapRe = (Map>>) barHandler .recover(new Object[] { "Aldo" }, new RuntimeException("Planned")); assertEquals("barRecoverNumberValue", recoverResponseMapRe.get("bar").get("bar").get(0.0)); - } @Test @@ -218,7 +222,6 @@ public void multipleQualifyingRecoverMethods() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); assertEquals(1, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); - } @Test @@ -227,7 +230,6 @@ public void multipleQualifyingRecoverMethodsWithNull() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned"))); - } @Test @@ -236,7 +238,6 @@ public void multipleQualifyingRecoverMethodsWithNoThrowable() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversNoThrowable(), foo); assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned"))); - } @Test @@ -245,7 +246,6 @@ public void multipleQualifyingRecoverMethodsReOrdered() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversReOrdered(), foo); assertEquals(3, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); - } @Test @@ -255,7 +255,6 @@ public void multipleQualifyingRecoverMethodsExtendsThrowable() { new MultipleQualifyingRecoversExtendsThrowable(), foo); assertEquals(2, handler.recover(new Object[] { "Kevin" }, new IllegalArgumentException("Planned"))); assertEquals(3, handler.recover(new Object[] { "Kevin" }, new UnsupportedOperationException("Planned"))); - } @Test @@ -363,7 +362,7 @@ public int foo(String name) { } @Recover - public int bar(RuntimeException e, String name) { + public int bar(IllegalStateException e, String name) { return 1; } diff --git a/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java b/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java index d56697d5..aa7cff8a 100644 --- a/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java +++ b/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,4 +137,24 @@ public void testParent() throws Exception { assertSame(context, child.getParent()); } + @Test + public void testRetryForException() { + Map, Boolean> map = new HashMap, Boolean>(); + map.put(RuntimeException.class, true); + SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true); + RetryContext context = policy.open(null); + assertNotNull(context); + assertTrue(policy.retryForException(new RuntimeException())); + } + + @Test + public void testNoRetryForException() { + Map, Boolean> map = new HashMap, Boolean>(); + map.put(IllegalArgumentException.class, true); + SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true); + RetryContext context = policy.open(null); + assertNotNull(context); + assertFalse(policy.retryForException(new RuntimeException())); + } + } diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 83959397..5edc68d1 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -346,7 +346,6 @@ public Object doWithRetry(RetryContext context) throws Exception { */ @Test public void testNoBackOffForRethrownException() throws Throwable { - RetryTemplate tested = new RetryTemplate(); tested.setRetryPolicy(new SimpleRetryPolicy(1)); @@ -384,6 +383,54 @@ public boolean rollbackFor(Throwable exception) { verify(bop); } + @Test + public void testRethrowForNotRetryable() throws Throwable { + SimpleRetryPolicy policy = new SimpleRetryPolicy(1, + Collections., Boolean>singletonMap(IllegalArgumentException.class, true)); + RetryTemplate retryTemplate = new RetryTemplate(); + retryTemplate.setRetryPolicy(policy); + retryTemplate.setNoRecoveryForNotRetryable(true); + try { + retryTemplate.execute(new RetryCallback() { + @Override + public Object doWithRetry(RetryContext context) throws Exception { + throw new RuntimeException("Realllly bad!"); + } + }, new RecoveryCallback() { + @Override + public Object recover(RetryContext context) throws Exception { + return new Object(); + } + }); + fail("Expected RuntimeException"); + } + catch (RuntimeException e) { + assertEquals("Realllly bad!", e.getMessage()); + } + } + + @Test + public void testRethrowForRetryable() throws Throwable { + SimpleRetryPolicy policy = new SimpleRetryPolicy(1, + Collections., Boolean>singletonMap(RuntimeException.class, true)); + RetryTemplate retryTemplate = new RetryTemplate(); + retryTemplate.setRetryPolicy(policy); + retryTemplate.setNoRecoveryForNotRetryable(true); + final Object value = new Object(); + Object result = retryTemplate.execute(new RetryCallback() { + @Override + public Object doWithRetry(RetryContext context) throws Exception { + throw new RuntimeException("Will be recovered"); + } + }, new RecoveryCallback() { + @Override + public Object recover(RetryContext context) throws Exception { + return value; + } + }); + assertEquals(value, result); + } + private static class MockRetryCallback implements RetryCallback { private int attempts;