From 1c7e9236bbeab2eefd2fcd528b4d83894dbebaa1 Mon Sep 17 00:00:00 2001 From: Akhmad Fathonih Date: Mon, 3 Oct 2022 10:23:18 +0900 Subject: [PATCH 1/2] use getRootCause to un-obscure root cause exception. --- graphwalker-core/pom.xml | 6 ++++- .../core/machine/ExecutionContext.java | 5 +++- .../java/test/TestExecutorTest.java | 23 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/graphwalker-core/pom.xml b/graphwalker-core/pom.xml index 7d4fe7808..d4410fdae 100644 --- a/graphwalker-core/pom.xml +++ b/graphwalker-core/pom.xml @@ -37,6 +37,10 @@ org.graalvm.truffle truffle-api - + + org.apache.commons + commons-lang3 + + diff --git a/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java b/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java index ec0a685d5..04f9b9281 100644 --- a/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java +++ b/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java @@ -26,6 +26,7 @@ * #L% */ +import org.apache.commons.lang3.exception.ExceptionUtils; import org.graalvm.polyglot.Value; import org.graphwalker.core.algorithm.Algorithm; import org.graphwalker.core.generator.PathGenerator; @@ -279,7 +280,9 @@ public void execute(Element element) { } catch (Throwable t) { executionStatus = ExecutionStatus.FAILED; LOG.error(t.getMessage()); - throw new MachineException(this, t); + // Do not obscure the root cause as it's not useful for the end user + // i.e: always showing java.lang.reflect.InvocationTargetException no matter what. + throw new MachineException(this, ExceptionUtils.getRootCause(t)); } } diff --git a/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java b/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java index a5edccfcd..8e1e29356 100644 --- a/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java +++ b/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java @@ -34,6 +34,7 @@ import org.graphwalker.core.statistics.Execution; import org.graphwalker.io.factory.json.JsonContextFactory; import org.graphwalker.java.annotation.GraphWalker; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; import org.junit.Test; @@ -194,6 +195,28 @@ public void throwExceptionExecutor() throws IOException { executor.execute(); } + /** + * Do not obscure the root cause + * @throws IOException + */ + @Test + public void doNotHideRootCauseException() throws IOException { + Executor executor = new TestExecutor(ThrowExceptionTest.class); + + Result result = executor.execute(true); + Assert.assertTrue(result.hasErrors()); + JSONArray failures = (JSONArray) result.getResults().get("failures"); + ArrayList failureList = new ArrayList<>(); + failures.forEach( failure -> { + Boolean hasFailureItem = ((JSONObject) failure).has("failure"); + if (hasFailureItem) { + failureList.add(((JSONObject) failure).getString("failure")); + } + } ); + + Assert.assertTrue(failureList.get(0).startsWith("java.lang.RuntimeException")); + } + @Test public void multilpeContexts() throws IOException { List contexts = new JsonContextFactory().create(Paths.get("org/graphwalker/java/test/PetClinic.json")); From 7e03f1eb4ff08540dee208b8714f81a973d24c37 Mon Sep 17 00:00:00 2001 From: Akhmad Fathonih Date: Mon, 3 Oct 2022 15:53:15 +0900 Subject: [PATCH 2/2] Unwrap more bubbles. --- .../core/machine/ExecutionContext.java | 2 +- .../java/test/TestExecutionException.java | 5 ++++ .../graphwalker/java/test/TestExecutor.java | 5 +++- .../java/test/TestExecutorTest.java | 29 +++++++++++++++++-- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java b/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java index 04f9b9281..01d2f7298 100644 --- a/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java +++ b/graphwalker-core/src/main/java/org/graphwalker/core/machine/ExecutionContext.java @@ -279,7 +279,7 @@ public void execute(Element element) { // ignore, method is not defined in the execution context } catch (Throwable t) { executionStatus = ExecutionStatus.FAILED; - LOG.error(t.getMessage()); + LOG.error(ExceptionUtils.getRootCauseMessage(t)); // Do not obscure the root cause as it's not useful for the end user // i.e: always showing java.lang.reflect.InvocationTargetException no matter what. throw new MachineException(this, ExceptionUtils.getRootCause(t)); diff --git a/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutionException.java b/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutionException.java index 21ab6963d..65077702f 100644 --- a/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutionException.java +++ b/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutionException.java @@ -41,6 +41,11 @@ public TestExecutionException(Result result) { this.result = result; } + public TestExecutionException(Result result, Throwable t) { + super(t); + this.result = result; + } + public TestExecutionException(String message) { super(message); } diff --git a/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutor.java b/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutor.java index 53a950d64..571c8d453 100644 --- a/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutor.java +++ b/graphwalker-java/src/main/java/org/graphwalker/java/test/TestExecutor.java @@ -26,6 +26,7 @@ * #L% */ +import org.apache.commons.lang3.exception.ExceptionUtils; import org.graphwalker.core.event.EventType; import org.graphwalker.core.event.Observer; import org.graphwalker.core.machine.Context; @@ -242,6 +243,7 @@ public Result execute() { public Result execute(boolean ignoreErrors) { result = new Result(); executeAnnotation(BeforeExecution.class, machine); + Throwable executionException = null; try { while (machine.hasNextStep()) { machine.getNextStep(); @@ -249,11 +251,12 @@ public Result execute(boolean ignoreErrors) { } catch (MachineException e) { logger.error(e.getMessage()); failures.put(e.getContext(), e); + executionException = e; } executeAnnotation(AfterExecution.class, machine); result.updateResults(machine, failures); if (!ignoreErrors && !failures.isEmpty()) { - throw new TestExecutionException(result); + throw new TestExecutionException(result, ExceptionUtils.getRootCause(executionException)); } return result; } diff --git a/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java b/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java index 8e1e29356..7b04c9e83 100644 --- a/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java +++ b/graphwalker-java/src/test/java/org/graphwalker/java/test/TestExecutorTest.java @@ -185,6 +185,12 @@ public ThrowExceptionTest() { } public void throwException() { + PossiblyAnActualRealWorldScenario.problematicMethod(); + } + } + + public static class PossiblyAnActualRealWorldScenario { + public static void problematicMethod() { throw new RuntimeException(); } } @@ -196,11 +202,11 @@ public void throwExceptionExecutor() throws IOException { } /** - * Do not obscure the root cause + * Do not obscure the root cause failure * @throws IOException */ @Test - public void doNotHideRootCauseException() throws IOException { + public void doNotObscureRootCauseFailureResult() throws IOException { Executor executor = new TestExecutor(ThrowExceptionTest.class); Result result = executor.execute(true); @@ -208,7 +214,7 @@ public void doNotHideRootCauseException() throws IOException { JSONArray failures = (JSONArray) result.getResults().get("failures"); ArrayList failureList = new ArrayList<>(); failures.forEach( failure -> { - Boolean hasFailureItem = ((JSONObject) failure).has("failure"); + boolean hasFailureItem = ((JSONObject) failure).has("failure"); if (hasFailureItem) { failureList.add(((JSONObject) failure).getString("failure")); } @@ -217,6 +223,23 @@ public void doNotHideRootCauseException() throws IOException { Assert.assertTrue(failureList.get(0).startsWith("java.lang.RuntimeException")); } + @Test + public void doNotObscureRootCauseStack() throws IOException { + Executor executor = new TestExecutor(ThrowExceptionTest.class); + Throwable t = null; + try { + executor.execute(); + } catch (Throwable thrown) { + t = thrown; + } + + Assert.assertNotNull(t); + Assert.assertTrue(Arrays.stream(t.getCause().getStackTrace()).anyMatch( stackTraceElement -> { + System.out.println(stackTraceElement.getMethodName()); + return stackTraceElement.getMethodName() == "problematicMethod"; + })); + } + @Test public void multilpeContexts() throws IOException { List contexts = new JsonContextFactory().create(Paths.get("org/graphwalker/java/test/PetClinic.json"));