BATCH-2213: unwrap exceptions thrown from annotated step listeners

Currently, when a method of an annotated listener throws an exception,
the exception is wrapped in a InvocationTargetException (by the reflection
API) which in turn is wrapped in a IllegalArgumentException (by Spring Batch).
This requires the user to unwrap the original exception from the
StepListenerFailedException. This behavior is not consistent with
interface based listeners where the original exception is the root
cause of StepListenerFailedException.

This commit unwraps the original exception and make it the root cause of
StepListenerFailedException.

Resolves BATCH-2213

Author: fmbenhassine

spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2013 the original author or authors.
+ * Copyright 2006-2018 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.
@@ -15,6 +15,7 @@
  */
 package org.springframework.batch.core.listener;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.List;
 
 import javax.batch.api.chunk.listener.RetryProcessListener;
@@ -37,6 +38,7 @@
  * @author Dave Syer
  * @author Michael Minella
  * @author Chris Schaefer
+ * @author Mahmoud Ben Hassine
  */
 public class MulticasterBatchListener<T, S> implements StepExecutionListener, ChunkListener, ItemReadListener<T>,
 ItemProcessListener<T, S>, ItemWriteListener<S>, SkipListener<T, S>, RetryReadListener, RetryProcessListener, RetryWriteListener {
@@ -133,7 +135,7 @@ public void afterProcess(T item, S result) {
 			itemProcessListener.afterProcess(item, result);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in afterProcess.", e);
+			throw new StepListenerFailedException("Error in afterProcess.", getTargetException(e));
 		}
 	}
 
@@ -146,7 +148,7 @@ public void beforeProcess(T item) {
 			itemProcessListener.beforeProcess(item);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in beforeProcess.", e);
+			throw new StepListenerFailedException("Error in beforeProcess.", getTargetException(e));
 		}
 	}
 
@@ -199,7 +201,7 @@ public void afterChunk(ChunkContext context) {
 			chunkListener.afterChunk(context);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in afterChunk.", e);
+			throw new StepListenerFailedException("Error in afterChunk.", getTargetException(e));
 		}
 	}
 
@@ -212,7 +214,7 @@ public void beforeChunk(ChunkContext context) {
 			chunkListener.beforeChunk(context);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in beforeChunk.", e);
+			throw new StepListenerFailedException("Error in beforeChunk.", getTargetException(e));
 		}
 	}
 
@@ -225,7 +227,7 @@ public void afterRead(T item) {
 			itemReadListener.afterRead(item);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in afterRead.", e);
+			throw new StepListenerFailedException("Error in afterRead.", getTargetException(e));
 		}
 	}
 
@@ -238,7 +240,7 @@ public void beforeRead() {
 			itemReadListener.beforeRead();
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in beforeRead.", e);
+			throw new StepListenerFailedException("Error in beforeRead.", getTargetException(e));
 		}
 	}
 
@@ -264,7 +266,7 @@ public void afterWrite(List<? extends S> items) {
 			itemWriteListener.afterWrite(items);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in afterWrite.", e);
+			throw new StepListenerFailedException("Error in afterWrite.", getTargetException(e));
 		}
 	}
 
@@ -277,7 +279,7 @@ public void beforeWrite(List<? extends S> items) {
 			itemWriteListener.beforeWrite(items);
 		}
 		catch (RuntimeException e) {
-			throw new StepListenerFailedException("Error in beforeWrite.", e);
+			throw new StepListenerFailedException("Error in beforeWrite.", getTargetException(e));
 		}
 	}
 
@@ -356,4 +358,17 @@ public void onRetryWriteException(List<Object> items, Exception ex) throws Excep
 			throw new BatchRuntimeException(e);
 		}
 	}
+
+	/**
+	 * Unwrap the target exception from a wrapped {@link InvocationTargetException}.
+	 * @param e the exception to introspect
+	 * @return the target exception if any
+	 */
+	private Throwable getTargetException(RuntimeException e) {
+		Throwable cause = e.getCause();
+		if (cause != null && cause instanceof InvocationTargetException) {
+			return ((InvocationTargetException) cause).getTargetException();
+		}
+		return e;
+	}
 }

spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2007 the original author or authors.
+ * Copyright 2006-2018 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.
@@ -16,6 +16,7 @@
 package org.springframework.batch.core.listener;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.util.Arrays;
@@ -26,10 +27,20 @@
 import org.springframework.batch.core.ExitStatus;
 import org.springframework.batch.core.JobExecution;
 import org.springframework.batch.core.StepExecution;
+import org.springframework.batch.core.StepListener;
+import org.springframework.batch.core.annotation.AfterChunk;
+import org.springframework.batch.core.annotation.AfterProcess;
+import org.springframework.batch.core.annotation.AfterRead;
+import org.springframework.batch.core.annotation.AfterWrite;
+import org.springframework.batch.core.annotation.BeforeChunk;
+import org.springframework.batch.core.annotation.BeforeProcess;
+import org.springframework.batch.core.annotation.BeforeRead;
+import org.springframework.batch.core.annotation.BeforeWrite;
 import org.springframework.batch.core.scope.context.ChunkContext;
 
 /**
  * @author Dave Syer
+ * @author Mahmoud Ben Hassine
  *
  */
 public class MulticasterBatchListenerTests {
@@ -512,6 +523,188 @@ public void onSkipInProcess(Object item, Throwable t) {
 		assertEquals(1, count);
 	}
 
+	@Test
+	public void testBeforeReadFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.beforeRead();
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testAfterReadFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.afterRead(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testBeforeProcessFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.beforeProcess(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testAfterProcessFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.afterProcess(null, null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testBeforeWriteFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.beforeWrite(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testAfterWriteFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.afterWrite(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testBeforeChunkFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.beforeChunk(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	@Test
+	public void testAfterChunkFails_withAnnotatedListener() {
+		StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener());
+		multicast.register(listener);
+
+		try {
+			multicast.afterChunk(null);
+			fail("Expected StepListenerFailedException");
+		} catch (StepListenerFailedException e) {
+			// expected
+			Throwable cause = e.getCause();
+			String message = cause.getMessage();
+			assertTrue(cause instanceof IllegalStateException);
+			assertEquals("Wrong message: " + message, "listener error", message);
+		}
+	}
+
+	private final class AnnotationBasedStepListener {
+
+		private IllegalStateException exception = new IllegalStateException("listener error");
+
+		@BeforeRead
+		public void beforeRead() {
+			throw exception;
+		}
+
+		@AfterRead
+		public void afterRead() {
+			throw exception;
+		}
+
+		@BeforeProcess
+		public void beforeProcess() {
+			throw exception;
+		}
+
+		@AfterProcess
+		public void afterProcess() {
+			throw exception;
+		}
+
+		@BeforeWrite
+		public void beforeWrite() {
+			throw exception;
+		}
+
+		@AfterWrite
+		public void afterWrite() {
+			throw exception;
+		}
+
+		@BeforeChunk
+		public void beforeChunk() {
+			throw exception;
+		}
+
+		@AfterChunk
+		public void afterChunk() {
+			throw exception;
+		}
+
+	}
+
 	/**
 	 * @author Dave Syer
 	 *