OPEN - issue BATCH-777: Parametrise RetryCallback and related interfaces

Downgrade throws clause in RetryCallback - it's not reasonable to retry arbitrary Throwable

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/RetryCallback.java

@@ -30,6 +30,7 @@ public interface RetryCallback {
 	 * semantics when an operation is retried.
 	 * @param context the current retry context.
 	 * @return the result of the successful operation.
+	 * @throws Exception TODO
 	 */
-	Object doWithRetry(RetryContext context) throws Throwable;
+	Object doWithRetry(RetryContext context) throws Exception;
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/callback/RecoveryRetryCallback.java

@@ -105,7 +105,7 @@ public class RecoveryRetryCallback implements RetryCallback {
 		this.forceRefresh = forceRefresh;
 	}
 
-	public Object doWithRetry(RetryContext context) throws Throwable {
+	public Object doWithRetry(RetryContext context) throws Exception {
 		return callback.doWithRetry(context);
 		// N.B. code used to check here for isExhaustedOnly and throw exception.
 		// This is unnecessary because the callback could just throw the

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/RetryOperationsInterceptor.java

@@ -54,7 +54,7 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
 
 		return this.retryOperations.execute(new RetryCallback() {
 
-			public Object doWithRetry(RetryContext context) throws Throwable {
+			public Object doWithRetry(RetryContext context) throws Exception {
 
 				/*
 				 * If we don't copy the invocation carefully it won't keep a
@@ -64,8 +64,17 @@ public class RetryOperationsInterceptor implements MethodInterceptor {
 				 * implementation come along?).
 				 */
 				if (invocation instanceof ProxyMethodInvocation) {
-					return ((ProxyMethodInvocation) invocation)
-							.invocableClone().proceed();
+					try {
+						return ((ProxyMethodInvocation) invocation)
+								.invocableClone().proceed();
+					}
+					catch (Exception e) {
+						throw e;
+					} catch (Error e) {
+						throw e;
+					} catch (Throwable e) {
+						throw new IllegalStateException(e);
+					}
 				} else {
 					throw new IllegalStateException(
 							"MethodInvocation of the wrong type detected - this should not happen with Spring AOP, so please raise an issue if you see this exception");

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/interceptor/StatefulRetryOperationsInterceptor.java

@@ -174,8 +174,19 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor {
 			this.invocation = invocation;
 		}
 
-		public Object doWithRetry(RetryContext context) throws Throwable {
-			return invocation.proceed();
+		public Object doWithRetry(RetryContext context) throws Exception {
+			try {
+				return invocation.proceed();
+			}
+			catch (Exception e) {
+				throw e;
+			}
+			catch (Error e) {
+				throw e;
+			}
+			catch (Throwable e) {
+				throw new IllegalStateException(e);
+			}
 		}
 	}
 

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java

@@ -24,7 +24,6 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.batch.retry.RetryCallback;
 import org.springframework.batch.retry.RetryContext;
-import org.springframework.batch.retry.RetryException;
 import org.springframework.batch.retry.RetryListener;
 import org.springframework.batch.retry.RetryOperations;
 import org.springframework.batch.retry.RetryPolicy;
@@ -167,17 +166,17 @@ public class RetryTemplate implements RetryOperations {
 					lastException = null;
 					return callback.doWithRetry(context);
 				}
-				catch (Throwable ex) {
-					Throwable throwable = unwrapIfRethrown(ex);
-					lastException = throwable;
+				catch (Exception e) {
 
-					doOnErrorInterceptors(callback, context, throwable);
+					lastException = e;
 
-					retryPolicy.registerThrowable(context, throwable);
+					doOnErrorInterceptors(callback, context, e);
+
+					retryPolicy.registerThrowable(context, e);
 
 					if (retryPolicy.shouldRethrow(context)) {
 						logger.debug("Rethrow in retry for policy: count=" + context.getRetryCount());
-						rethrow(throwable);
+						throw e;
 					}
 
 				}
@@ -190,7 +189,7 @@ public class RetryTemplate implements RetryOperations {
 					// back off was prevented by another thread - fail the
 					// retry
 					logger.debug("Abort retry because interrupted: count=" + context.getRetryCount());
-					rethrow(e);
+					throw e;
 				}
 
 				/*
@@ -235,44 +234,4 @@ public class RetryTemplate implements RetryOperations {
 		}
 	}
 
-	/**
-	 * Re-throw the exception directly if possible, wrap custom Throwables into
-	 * {@link UnclassifiedRetryException}.
-	 */
-	private static void rethrow(Throwable throwable) throws Exception {
-		if (throwable instanceof Exception) {
-			throw (Exception) throwable;
-		}
-		else if (throwable instanceof Error) {
-			throw (Error) throwable;
-		}
-		else {
-			throw new UnclassifiedRetryException("Unclassified Throwable encountered", throwable);
-		}
-	}
-
-	/**
-	 * Undo the wrapping done in {@link #rethrow(Throwable)}
-	 */
-	private static Throwable unwrapIfRethrown(Throwable throwable) {
-		if (throwable instanceof UnclassifiedRetryException) {
-			return throwable.getCause();
-		}
-		else {
-			return throwable;
-		}
-	}
-
-	/**
-	 * Runtime exception wrapper for Throwables that are neither Exception nor
-	 * Error.
-	 */
-	private static class UnclassifiedRetryException extends RetryException {
-
-		public UnclassifiedRetryException(String msg, Throwable cause) {
-			super(msg, cause);
-		}
-
-	}
-
 }