SPR-6941: part (1) correct exception handling when null cachedExecutor

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

@@ -89,17 +89,7 @@ public class MethodReference extends SpelNodeImpl {
 				// may or may not be a root cause.  If there is a root cause it is a user created exception.
 				// If there is no root cause it was a reflective invocation problem.
 				
-				Throwable causeOfAccessException = ae.getCause();
+				throwSimpleExceptionIfPossible(state, ae);
-				Throwable rootCause = (causeOfAccessException==null?null:causeOfAccessException.getCause());
-				if (rootCause!=null) {
-					// User exception was the root cause - exit now
-					if (rootCause instanceof RuntimeException) {
-						throw (RuntimeException)rootCause;
-					} else {
-						throw new SpelEvaluationException( getStartPosition(), rootCause, SpelMessage.EXCEPTION_DURING_METHOD_INVOCATION,
-							this.name, state.getActiveContextObject().getValue().getClass().getName(), rootCause.getMessage());
-					}
-				}
 				
 				// at this point we know it wasn't a user problem so worth a retry if a better candidate can be found
 				this.cachedExecutor = null;
@@ -113,11 +103,32 @@ public class MethodReference extends SpelNodeImpl {
 			return executorToUse.execute(
 					state.getEvaluationContext(), state.getActiveContextObject().getValue(), arguments);
 		} catch (AccessException ae) {
+			// Same unwrapping exception handling as above in above catch block
+			throwSimpleExceptionIfPossible(state, ae);
 			throw new SpelEvaluationException( getStartPosition(), ae, SpelMessage.EXCEPTION_DURING_METHOD_INVOCATION,
 					this.name, state.getActiveContextObject().getValue().getClass().getName(), ae.getMessage());
 		}
 	}
 
+
+	/**
+	 * Decode the AccessException, throwing a lightweight evaluation exception or, if the cause was a RuntimeException, 
+	 * throw the RuntimeException directly.
+	 */
+	private void throwSimpleExceptionIfPossible(ExpressionState state, AccessException ae) {
+		Throwable causeOfAccessException = ae.getCause();
+		Throwable rootCause = (causeOfAccessException==null?null:causeOfAccessException.getCause());
+		if (rootCause!=null) {
+			// User exception was the root cause - exit now
+			if (rootCause instanceof RuntimeException) {
+				throw (RuntimeException)rootCause;
+			} else {
+				throw new SpelEvaluationException( getStartPosition(), rootCause, SpelMessage.EXCEPTION_DURING_METHOD_INVOCATION,
+					this.name, state.getActiveContextObject().getValue().getClass().getName(), rootCause.getMessage());
+			}
+		}
+	}
+
 	private Class<?>[] getTypes(Object... arguments) {
 		Class<?>[] argumentTypes = new Class[arguments.length];
 		for (int i = 0; i < arguments.length; i++) {

org.springframework.expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java

@@ -156,6 +156,33 @@ public class MethodInvocationTests extends ExpressionTestCase {
 		Assert.assertEquals(4,parser.parseExpression("counter").getValue(eContext));
 	}
 	
+	/**
+	 * Check on first usage (when the cachedExecutor in MethodReference is null) that the exception is not wrapped.
+	 */
+	@Test
+	public void testMethodThrowingException_SPR6941() {
+		// Test method on inventor: throwException()
+		// On 1 it will throw an IllegalArgumentException
+		// On 2 it will throw a RuntimeException
+		// On 3 it will exit normally
+		// In each case it increments the Inventor field 'counter' when invoked
+		
+		SpelExpressionParser parser = new SpelExpressionParser();
+		Expression expr = parser.parseExpression("throwException(#bar)");
+		
+		eContext.setVariable("bar",2);
+		try {
+			expr.getValue(eContext);
+			Assert.fail();
+		} catch (Exception e) {
+			if (e instanceof SpelEvaluationException) {
+				e.printStackTrace();
+				Assert.fail("Should not be a SpelEvaluationException");
+			}
+			// normal
+		}
+	}
+	
 	@Test
 	public void testMethodFiltering_SPR6764() {
 		SpelExpressionParser parser = new SpelExpressionParser();