8247438: JShell: When FailOverExecutionControlProvider fails the proximal cause is not shown

8237743: test/langtools/jdk/jshell/FailOverExecutionControlTest.java fails No ExecutionControlProvider with name 'nonExistent' and parameter keys: []
8199646: JShell tests: jdk/jshell/FailOverDirectExecutionControlTest.java failed with java.lang.UnsupportedOperationException

Reviewed-by: jlahoda

src/jdk.jshell/share/classes/jdk/jshell/execution/FailOverExecutionControlProvider.java

@@ -96,6 +96,8 @@ public class FailOverExecutionControlProvider  implements ExecutionControlProvid
     public ExecutionControl generate(ExecutionEnv env, Map<String, String> parameters)
             throws Throwable {
         Throwable thrown = null;
+        StringWriter dumpsw = new StringWriter();
+        PrintWriter dump = new PrintWriter(dumpsw);
         for (int i = 0; i <= 9; ++i) {
             String param = parameters.get("" + i);
             if (param != null && !param.isEmpty()) {
@@ -115,10 +117,17 @@ public class FailOverExecutionControlProvider  implements ExecutionControlProvid
                     ex.printStackTrace(log);
                     log.flush();
                     logger().fine(writer.toString());
-                    // only care about the first, and only if they all fail
-                    if (thrown == null) {
-                        thrown = ex;
+                    // if they all fail, use the last as cause and include info about prior in message
+                    dump.printf("FailOverExecutionControlProvider: FAILED: %d:%s --%n", i, param);
+                    dump.printf("  Exception: %s%n", ex);
+                    var st = ex.getStackTrace();
+                    for (int k = 0; k < 5 && k < st.length; ++k) {
+                        dump.printf("                  %s%n", st[k]);
                     }
+                    if (ex.getCause() != null) {
+                        dump.printf("      cause: %s%n", ex.getCause());
+                    }
+                    thrown = ex;
                 }
             }
 
@@ -127,7 +136,7 @@ public class FailOverExecutionControlProvider  implements ExecutionControlProvid
         if (thrown == null) {
             throw new IllegalArgumentException("All least one parameter must be set to a provider.");
         }
-        throw thrown;
+        throw new RuntimeException(dumpsw.toString(), thrown);
     }
 
     private Logger logger() {

test/langtools/jdk/jshell/ExecutionControlTestBase.java

@@ -25,12 +25,30 @@ import javax.tools.Diagnostic;
 
 import org.testng.annotations.Test;
 import jdk.jshell.VarSnippet;
+import java.net.InetAddress;
 
 import static jdk.jshell.Snippet.Status.VALID;
 import static jdk.jshell.Snippet.SubKind.*;
 
 public class ExecutionControlTestBase extends KullaTesting {
 
+    String standardListenSpec() {
+        String loopback = InetAddress.getLoopbackAddress().getHostAddress();
+        return "jdi:hostname(" + loopback + ")";
+    }
+
+    String standardLaunchSpec() {
+        return "jdi:launch(true)";
+    }
+
+    String standardJdiSpec() {
+        return "jdi";
+    }
+
+    String standardSpecs() {
+        return "5(" + standardListenSpec() + "), 6(" + standardLaunchSpec() + "), 7(" + standardJdiSpec() + ")";
+    }
+
     @Test
     public void classesDeclaration() {
         assertEval("interface A { }");

test/langtools/jdk/jshell/FailOverDirectExecutionControlTest.java

@@ -129,7 +129,9 @@ public class FailOverDirectExecutionControlTest extends ExecutionControlTestBase
         Map<String, String> pm = provider.defaultParameters();
         pm.put("0", "alwaysFailing");
         pm.put("1", "alwaysFailing");
-        pm.put("2", "jdi");
+        pm.put("2", standardListenSpec());
+        pm.put("3", standardLaunchSpec());
+        pm.put("4", standardJdiSpec());
         setUp(builder -> builder.executionEngine(provider, pm));
     }
 
@@ -156,6 +158,10 @@ public class FailOverDirectExecutionControlTest extends ExecutionControlTestBase
         assertTrue(log.contains("Failure failover -- 1 = alwaysFailing"), log);
         assertTrue(log.contains("This operation intentionally broken"), log);
         log = logged.get(Level.FINEST).get(0);
-        assertTrue(log.contains("Success failover -- 2 = jdi"), log);
+        assertTrue(
+                log.contains("Success failover -- 2 = " + standardListenSpec())
+                || log.contains("Success failover -- 3 = " + standardLaunchSpec())
+                || log.contains("Success failover -- 4 = " + standardJdiSpec()),
+                log);
     }
 }

test/langtools/jdk/jshell/FailOverExecutionControlDyingLaunchTest.java

@@ -42,6 +42,7 @@ public class FailOverExecutionControlDyingLaunchTest extends ExecutionControlTes
     @Override
     public void setUp() {
         setUp(builder -> builder.executionEngine(
-                "failover:0(jdi:remoteAgent(DyingRemoteAgent),launch(true)), 4(jdi:launch(true))"));
+                "failover:0(jdi:remoteAgent(DyingRemoteAgent),launch(true)), "
+                    + standardSpecs()));
     }
 }

test/langtools/jdk/jshell/FailOverExecutionControlHangingLaunchTest.java

@@ -41,6 +41,7 @@ public class FailOverExecutionControlHangingLaunchTest extends ExecutionControlT
     @Override
     public void setUp() {
         setUp(builder -> builder.executionEngine(
-                "failover:0(jdi:remoteAgent(HangingRemoteAgent),launch(true)), 1(jdi:launch(true))"));
+                "failover:0(jdi:remoteAgent(HangingRemoteAgent),launch(true)), "
+                        + standardSpecs()));
     }
 }

test/langtools/jdk/jshell/FailOverExecutionControlHangingListenTest.java

@@ -45,6 +45,6 @@ public class FailOverExecutionControlHangingListenTest extends ExecutionControlT
         String loopback = InetAddress.getLoopbackAddress().getHostAddress();
          setUp(builder -> builder.executionEngine(
                 "failover:0(jdi:remoteAgent(HangingRemoteAgent),hostname(" + loopback + ")),"
-                 + "1(jdi:hostname(" + loopback + "))"));
+                        + standardSpecs()));
     }
 }

test/langtools/jdk/jshell/FailOverExecutionControlTest.java

@@ -40,7 +40,8 @@ public class FailOverExecutionControlTest extends ExecutionControlTestBase {
     @BeforeMethod
     @Override
     public void setUp() {
-        setUp(builder -> builder.executionEngine("failover:0(nonExistent), 1(nonExistent), 2(jdi:launch(true))"));
+        setUp(builder -> builder.executionEngine("failover:0(expectedFailureNonExistent1), 1(expectedFailureNonExistent2), "
+                + standardSpecs()));
     }
 
 }