Repository: cloudslang/cloud-slang
Issue: Join branches throws NPE when aggregating wrong output

when running flow:
https://gist.github.com/orius123/a5c2e9facf20e6d0de00

NPE is thrown from AsyncLoopSteps
my best guess is that something in this chain in null
[Relevant line](https://github.com/CloudSlang/cloud-slang/blob/7c2ff9c81ad706e065bf394f890b31c55ad6c038/cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java#L224)

10x @genadi-hp for catching this issue


Changed files:
cloudslang-runtime/src/test/java/io/cloudslang/lang/runtime/steps/AsyncLoopStepsTest.java
cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java

diff --git a/cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java b/cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java
index 46d486239..f7180b930 100644
--- a/cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java
+++ b/cloudslang-runtime/src/main/java/io/cloudslang/lang/runtime/steps/AsyncLoopSteps.java
@@ -24,7 +24,10 @@ import io.cloudslang.lang.runtime.events.LanguageEventData;
 import io.cloudslang.score.api.EndBranchDataContainer;
 import io.cloudslang.score.api.execution.ExecutionParametersConsts;
 import io.cloudslang.score.lang.ExecutionRuntimeServices;
+import io.cloudslang.score.lang.SystemContext;
+import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.lang.SerializationUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.log4j.Logger;
 import org.python.google.common.collect.Lists;
@@ -46,6 +49,8 @@ import static io.cloudslang.score.api.execution.ExecutionParametersConsts.EXECUT
 @Component
 public class AsyncLoopSteps extends AbstractSteps {
 
+    public static final String BRANCH_EXCEPTION_PREFIX = "Error running branch";
+
     @Autowired
     private AsyncLoopBinding asyncLoopBinding;
 
@@ -128,7 +133,7 @@ public class AsyncLoopSteps extends AbstractSteps {
             Map<String, Serializable> contextBeforeSplit = flowContext.getImmutableViewOfVariables();
             List<String> branchesResult = Lists.newArrayList();
 
-            collectBranchesData(runEnv, executionRuntimeServices, nodeName, branchesContext, branchesResult);
+            collectBranchesData(executionRuntimeServices, nodeName, branchesContext, branchesResult);
 
             Map<String, Serializable> publishValues =
                     bindAggregateOutputs(
@@ -150,7 +155,7 @@ public class AsyncLoopSteps extends AbstractSteps {
             runEnv.getExecutionPath().forward();
         } catch (RuntimeException e) {
             logger.error("There was an error running the end task execution step of: \\'" + nodeName + "\\'. Error is: " + e.getMessage());
-            throw new RuntimeException("Error running: \\'" + nodeName + "\\': " + e.getMessage(), e);
+            throw new RuntimeException("Error running: \\'" + nodeName + "\\': \\n" + e.getMessage(), e);
         }
     }
 
@@ -219,7 +224,6 @@ public class AsyncLoopSteps extends AbstractSteps {
     }
 
     private void collectBranchesData(
-            RunEnvironment runEnv,
             ExecutionRuntimeServices executionRuntimeServices,
             String nodeName,
             List<Map<String, Serializable>> branchesContext,
@@ -227,6 +231,20 @@ public class AsyncLoopSteps extends AbstractSteps {
 
         List<EndBranchDataContainer> branches = executionRuntimeServices.getFinishedChildBranchesData();
         for (EndBranchDataContainer branch : branches) {
+
+            //first we check that no exception was thrown during the execution of the branch
+            String branchException = branch.getException();
+            if (StringUtils.isNotEmpty(branchException)) {
+                Map<String, Serializable> systemContextMap = branch.getSystemContext();
+                String branchID = null;
+                if (MapUtils.isNotEmpty(systemContextMap)) {
+                    ExecutionRuntimeServices branchExecutionRuntimeServices = new SystemContext(systemContextMap);
+                    branchID = branchExecutionRuntimeServices.getBranchId();
+                }
+                logger.error("There was an error running branch: " + branchID + " Error is: " + branchException);
+                throw new RuntimeException(BRANCH_EXCEPTION_PREFIX + ": \\n" + branchException);
+            }
+
             Map<String, Serializable> branchContext = branch.getContexts();
 
             RunEnvironment branchRuntimeEnvironment = (RunEnvironment) branchContext.get(ScoreLangConstants.RUN_ENV);
diff --git a/cloudslang-runtime/src/test/java/io/cloudslang/lang/runtime/steps/AsyncLoopStepsTest.java b/cloudslang-runtime/src/test/java/io/cloudslang/lang/runtime/steps/AsyncLoopStepsTest.java
index 1048b164c..38aba6912 100644
--- a/cloudslang-runtime/src/test/java/io/cloudslang/lang/runtime/steps/AsyncLoopStepsTest.java
+++ b/cloudslang-runtime/src/test/java/io/cloudslang/lang/runtime/steps/AsyncLoopStepsTest.java
@@ -37,6 +37,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 
 import javax.script.ScriptEngine;
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -53,6 +54,8 @@ import static org.mockito.Mockito.*;
 @ContextConfiguration(classes = AsyncLoopStepsTest.Config.class)
 public class AsyncLoopStepsTest {
 
+    public static final String BRANCH_EXCEPTION_MESSAGE = "Exception details placeholder";
+
     @Autowired
     private AsyncLoopSteps asyncLoopSteps;
 
@@ -390,6 +393,29 @@ public class AsyncLoopStepsTest {
         Assert.assertEquals(expectedEventTypesInOrder, actualEventTypesInOrder);
     }
 
+    @Test
+    public void testExceptionIsCapturedFromBranches() throws Exception {
+        // prepare arguments
+        RunEnvironment runEnvironment = new RunEnvironment();
+        runEnvironment.getExecutionPath().down();
+        Map<String, Serializable> variables = new HashMap<>();
+        Context context = new Context(variables);
+        runEnvironment.getStack().pushContext(context);
+
+        ExecutionRuntimeServices executionRuntimeServices = createExecutionRuntimeServicesMockWithBranchException();
+
+        exception.expectMessage(BRANCH_EXCEPTION_MESSAGE);
+        exception.expect(RuntimeException.class);
+
+        asyncLoopSteps.joinBranches(
+                runEnvironment,
+                executionRuntimeServices,
+                new ArrayList<Output>(0),
+                new HashMap<String, ResultNavigation>(),
+                "nodeName"
+        );
+    }
+
     private ExecutionRuntimeServices createAndConfigureExecutionRuntimeServicesMock(
             Map<String, Serializable> runtimeContext1,
             Map<String, Serializable> runtimeContext2,
@@ -445,6 +471,18 @@ public class AsyncLoopStepsTest {
         return executionRuntimeServices;
     }
 
+    private ExecutionRuntimeServices createExecutionRuntimeServicesMockWithBranchException() {
+        ExecutionRuntimeServices executionRuntimeServices = mock(ExecutionRuntimeServices.class);
+        List<EndBranchDataContainer> branchesContainers = Lists.newArrayList(
+                new EndBranchDataContainer(
+                        new HashMap<String, Serializable>(),
+                        new HashMap<String, Serializable>(),
+                        BRANCH_EXCEPTION_MESSAGE)
+        );
+        when(executionRuntimeServices.getFinishedChildBranchesData()).thenReturn(branchesContainers);
+        return executionRuntimeServices;
+    }
+
     @Configuration
     static class Config {