From d3b6491bbcb43f9445dfebd5a7aebeed77f5975a Mon Sep 17 00:00:00 2001
From: Janne Valkealahti <janne.valkealahti@gmail.com>
Date: Fri, 5 May 2017 14:55:56 +0100
Subject: [PATCH] Fix action error in anonymous transition

- Executor is following anonymous transitions in a loop
  and if there is an exception in actions with this transition,
  effectively executor ended into infinite loop.
- No bailing out from this loop if we cannot continue.
- Backport #344
- Relates #307
---
 .../support/AbstractStateMachine.java         |  5 +-
 .../support/DefaultStateMachineExecutor.java  |  8 ++-
 .../statemachine/action/ActionErrorTests.java | 51 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java
index bbb59208..35742c2f 100644
--- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java
+++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java
@@ -29,6 +29,7 @@ import org.springframework.statemachine.StateContext;
 import org.springframework.statemachine.StateContext.Stage;
 import org.springframework.statemachine.StateMachine;
 import org.springframework.statemachine.StateMachineContext;
+import org.springframework.statemachine.StateMachineException;
 import org.springframework.statemachine.access.StateMachineAccess;
 import org.springframework.statemachine.access.StateMachineAccessor;
 import org.springframework.statemachine.access.StateMachineFunction;
@@ -310,8 +311,10 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
 				try {
 					t.executeTransitionActions(ctx);
 				} catch (Exception e) {
+					// aborting, executor should stop possible loop checking possible transitions
+					// causing infinite execution
 					log.warn("Aborting as transition " + t + " caused error " + e);
-					return;
+					throw new StateMachineException("Aborting as transition " + t + " caused error ", e);
 				}
 				notifyTransition(buildStateContext(Stage.TRANSITION, message, t, getRelayStateMachine()));
 				if (t.getTarget().getPseudoState() != null && t.getTarget().getPseudoState().getKind() == PseudoStateKind.JOIN) {
diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java
index 27f64d66..2fd87fd8 100644
--- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java
+++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/support/DefaultStateMachineExecutor.java
@@ -243,7 +243,13 @@ public class DefaultStateMachineExecutor<S, E> extends LifecycleObjectSupport im
 				log.warn("Aborting as transition " + t + " caused error " + e);
 			}
 			if (transit) {
-				stateMachineExecutorTransit.transit(t, stateContext, queuedMessage);
+				// if executor transit is raising exception, stop here
+				try {
+					stateMachineExecutorTransit.transit(t, stateContext, queuedMessage);
+				} catch (Exception e) {
+					interceptors.postTransition(stateContext);
+					return false;
+				}
 				interceptors.postTransition(stateContext);
 				break;
 			}
diff --git a/spring-statemachine-core/src/test/java/org/springframework/statemachine/action/ActionErrorTests.java b/spring-statemachine-core/src/test/java/org/springframework/statemachine/action/ActionErrorTests.java
index a4267166..2cb94d68 100644
--- a/spring-statemachine-core/src/test/java/org/springframework/statemachine/action/ActionErrorTests.java
+++ b/spring-statemachine-core/src/test/java/org/springframework/statemachine/action/ActionErrorTests.java
@@ -54,6 +54,21 @@ public class ActionErrorTests extends AbstractStateMachineTests {
 		assertThat(machine.getState().getIds(), contains(TestStates.S1));
 	}
 
+	@Test
+	public void testActionExceptionInAnonymousTransition() {
+		context.register(Config2.class);
+		context.refresh();
+		assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE));
+		@SuppressWarnings("unchecked")
+		ObjectStateMachine<TestStates,TestEvents> machine =
+				context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class);
+		machine.start();
+		// error in transition should not cause transition and should
+		// not propagate error into a caller.
+		machine.sendEvent(TestEvents.E1);
+		assertThat(machine.getState().getIds(), contains(TestStates.S2));
+	}
+
 	@Configuration
 	@EnableStateMachine
 	static class Config1 extends EnumStateMachineConfigurerAdapter<TestStates, TestEvents> {
@@ -84,6 +99,42 @@ public class ActionErrorTests extends AbstractStateMachineTests {
 
 	}
 
+	@Configuration
+	@EnableStateMachine
+	static class Config2 extends EnumStateMachineConfigurerAdapter<TestStates, TestEvents> {
+
+		@Override
+		public void configure(StateMachineStateConfigurer<TestStates, TestEvents> states) throws Exception {
+			states
+				.withStates()
+					.initial(TestStates.S1)
+					.state(TestStates.S2)
+					.end(TestStates.SF);
+		}
+
+		@Override
+		public void configure(StateMachineTransitionConfigurer<TestStates, TestEvents> transitions) throws Exception {
+			transitions
+				.withExternal()
+					.source(TestStates.S1)
+					.target(TestStates.S2)
+					.event(TestEvents.E1)
+					.and()
+				.withExternal()
+					.source(TestStates.S2)
+					.target(TestStates.SF)
+					.action(context -> {
+						throw new NullPointerException("Something was wrong");
+					});
+		}
+
+		@Bean
+		public TestCountAction testAction1() {
+			return new TestCountAction();
+		}
+
+	}
+
 	private static class TestCountAction implements Action<TestStates, TestEvents> {
 
 		@Override