runtime-v2: do not log stack trace for EL MethodNotFound exception an…

…d unify the error messages (#1076)
walmartlabs · Feb 9, 2025 · 445168c · 445168c
1 parent d41f8aa
commit 445168c
Show file tree

Hide file tree

Showing 16 changed files with 188 additions and 129 deletions.
diff --git a/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java b/common/src/main/java/com/walmartlabs/concord/common/ExceptionUtils.java
@@ -9,9 +9,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -36,12 +36,17 @@ public static List<Throwable> getExceptionList(Throwable e) {
     }
 
     @SuppressWarnings("unchecked")
-    public static <T> T filterException(Exception e, Class<T> clazz) {
-        return getExceptionList(e).stream()
-                .filter(clazz::isInstance)
-                .map(c -> (T)c)
-                .findAny()
-                .orElse(null);
+    public static <T extends Throwable> T findLastException(T e, Class<T> clazz) {
+        var exceptions = getExceptionList(e);
+
+        for (int i = exceptions.size() - 1; i >= 0; i--) {
+            var ex = exceptions.get(i);
+            if (clazz.isInstance(ex)) {
+                return (T) ex;
+            }
+        }
+
+        return e;
     }
 
     private ExceptionUtils() {

diff --git a/...unner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java b/...unner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogExceptionsTest.java
@@ -50,7 +50,7 @@ public void shouldLogExceptionStackTraceWhenTaskThrowsException() throws Excepti
         // error
         assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. boom!") + ".*");
         // stacktrace
-        assertLog(runtime.lastLog(), ".*at com.walmartlabs.concord.runtime.v2.runner.LogExceptionsTest.shouldLogExceptionStackTraceWhenTaskThrowsException.*");
+        assertLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask2.execute") + ".*");
     }
 
     @Test
@@ -69,12 +69,13 @@ public void shouldLogExceptionStackTraceWhenExpressionThrowsException() throws E
 
         // error
         // TODO: "javax.el.ELException: java.lang.Exception: ..." remove javax.el.ELException?
-        assertLog(runtime.lastLog(), ".*" + quote("Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': javax.el.ELException: java.lang.Exception: BOOM") + ".*");
+        assertLog(runtime.lastLog(), ".*" + quote("Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': BOOM") + ".*");
         // stacktrace
-        assertLog(runtime.lastLog(), ".*at com.walmartlabs.concord.runtime.v2.runner.LogExceptionsTest.shouldLogExceptionStackTraceWhenExpressionThrowsException.*");
+        assertLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask") + ".*");
     }
 
     @Test
+    @IgnoreSerializationAssert
     public void shouldLogExceptionStackTraceWhenTaskThrowsExceptionFromParallel() throws Exception {
         runtime.deploy("logExceptionTests/fromParallel");
 
@@ -144,6 +145,7 @@ public void noStacktraceForUserDefinedExceptionFromExpression() throws Exception
     }
 
     @Test
+    @IgnoreSerializationAssert
     public void noStacktraceForUserDefinedExceptionFromTaskParallel() throws Exception {
         runtime.deploy("logExceptionTests/userDefinedExceptionFromTaskParallel");
 
@@ -190,4 +192,46 @@ public void noStacktraceForTaskFailReturn() throws Exception {
         assertNoLog(runtime.lastLog(), ".*noStacktraceForTaskFailReturn.*");
         assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask.execute") + ".*");
     }
+
+    @Test
+    public void noStackTraceForMethodNotFound() throws Exception {
+        runtime.deploy("logExceptionTests/methodNotFound");
+
+        runtime.save(ProcessConfiguration.builder()
+                .build());
+
+        try {
+            runtime.run();
+            fail("Exception expected");
+        } catch (Exception e) {
+            // ignore
+        }
+
+        // error
+        assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.unknownMethod('BOOM')}': Can't find 'unknownMethod()' method in com.walmartlabs.concord.runtime.v2.runner.tasks.Tasks$FaultyTask") + ".*");
+
+        // no stacktrace
+        assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.el.LazyExpressionEvaluator.evalExpr") + ".*");
+    }
+
+    @Test
+    public void noStackTraceForVariableNotFound() throws Exception {
+        runtime.deploy("logExceptionTests/variableNotFound");
+
+        runtime.save(ProcessConfiguration.builder()
+                .build());
+
+        try {
+            runtime.run();
+            fail("Exception expected");
+        } catch (Exception e) {
+            // ignore
+        }
+
+        // error
+        assertLog(runtime.lastLog(), ".*" + quote("(concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${unknown}': Can't find a variable 'unknown'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'unknown'") + ".*");
+
+        // no stacktrace
+        assertNoLog(runtime.lastLog(), ".*" + quote("at com.walmartlabs.concord.runtime.v2.runner.el.LazyExpressionEvaluator.evalExpr") + ".*");
+    }
 }
diff --git a/.../runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java b/.../runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/LogSegmentsTest.java
@@ -82,6 +82,7 @@ public void throwStepShouldContainErrorDescription() throws Exception {
     }
 
     @Test
+    @IgnoreSerializationAssert
     public void loopStepShouldLogErrorInProperLogSegment() throws Exception {
         deploy("logSegments/taskErrorWithLoop");
 
@@ -256,7 +257,7 @@ public void taskOutInvalidTest() throws Exception {
 
         // invalid task out definition -> log into task segment
         assertSegmentLog(runtime.lastLog(), 1, "[INFO ] test");
-        assertSegmentLog(runtime.lastLog(), 1, "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSegmentLog(runtime.lastLog(), 1, "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
         assertSegmentStatusError(runtime.lastLog(), 1);
 
         assertNoMoreSegments();
@@ -276,7 +277,7 @@ public void taskLoopInvalidTest() throws Exception {
             // ignore
         }
 
-        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
 
         assertNoMoreSegments();
     }
@@ -389,7 +390,7 @@ public void taskWithInvalidName() throws Exception {
             // ignore
         }
 
-        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 4, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 4, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
         assertNoMoreSegments();
     }
 
@@ -431,7 +432,7 @@ public void taskWithRetryInvalid() throws Exception {
         }
 
         // logs to the system segment
-        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
         assertNoMoreSegments();
     }
 
@@ -451,7 +452,7 @@ public void checkpointInvalidTest() throws Exception {
         }
 
         // error into system segment
-        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
     }
 
     @Test
@@ -499,7 +500,7 @@ public void exprInvalidTest() throws Exception {
         }
 
         // error into system segment
-        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. Can't find a variable 'undefined' used in '${undefined}'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
+        assertSystemSegment(runtime.lastLog(), "[ERROR] (concord.yaml): Error @ line: 3, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'");
     }
 
     @Test

diff --git a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java
@@ -508,7 +508,7 @@ public void testScriptErrorBlock() throws Exception {
                 .build());
 
         byte[] log = run();
-        assertLog(log, ".*error occurred: java.lang.RuntimeException: Error: this is an error.*");
+        assertLog(log, ".*" + quote("(concord.yml): Error @ line: 3, col: 7. Error: this is an error") + ".*");
     }
 
     @Test
@@ -1412,7 +1412,7 @@ public void testInvalidExpressionError() throws Exception {
         }
 
         String logString = new String(runtime.lastLog());
-        String expected = "[ERROR] (concord.yml): Error @ line: 9, col: 7. Error Parsing: ${str.split('\\n')}. Encountered \"\\'\\\\n\" at line 1, column 13.\n" +
+        String expected = "[ERROR] (concord.yml): Error @ line: 9, col: 7. while parsing expression '${str.split('\\n')}': Encountered \"\\'\\\\n\" at line 1, column 13.\n" +
                 "Was expecting one of:\n" +
                 "    \"{\" ...\n" +
                 "    <INTEGER_LITERAL> ...\n" +
@@ -1483,7 +1483,7 @@ public void testExpressionThrowException() throws Exception {
             // ignore
         }
 
-        String expected = "[ERROR] (concord.yml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': javax.el.ELException: java.lang.Exception: BOOM";
+        String expected = "[ERROR] (concord.yml): Error @ line: 3, col: 7. while evaluating expression '${faultyTask.exception('BOOM')}': BOOM";
         assertLog(runtime.lastLog(), ".*" + Pattern.quote(expected));
     }
 
@@ -1517,7 +1517,7 @@ public void testUnresolvedVarInStepName() throws Exception {
         } catch (Exception e) {
         }
 
-        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 4, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*");
+        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 4, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*");
     }
 
     @Test
@@ -1533,7 +1533,7 @@ public void testUnresolvedVarInLoop() throws Exception {
         } catch (Exception e) {
         }
 
-        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*");
+        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*");
     }
 
     @Test
@@ -1549,7 +1549,7 @@ public void testUnresolvedVarInRetry() throws Exception {
         } catch (Exception e) {
         }
 
-        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. Can't find a variable 'undefined' used in '${undefined}'") + ".*");
+        assertLog(runtime.lastLog(), ".*" + quote("(concord.yml): Error @ line: 6, col: 7. while evaluating expression '${undefined}': Can't find a variable 'undefined'. Check if it is defined in the current scope. Details: ELResolver cannot handle a null base Object with identifier 'undefined'") + ".*");
     }
 
     @Test

diff --git a/...s/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml b/...s/com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/methodNotFound/concord.yaml
@@ -0,0 +1,3 @@
+flows:
+  default:
+    - expr: "${faultyTask.unknownMethod('BOOM')}"
diff --git a/...com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml b/...com/walmartlabs/concord/runtime/v2/runner/logExceptionTests/variableNotFound/concord.yaml
@@ -0,0 +1,3 @@
+flows:
+  default:
+    - expr: "${unknown}"
diff --git a/...me/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java b/...me/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/el/LazyEvalList.java
@@ -9,9 +9,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -27,10 +27,10 @@ public class LazyEvalList extends AbstractList<Object> {
     private final LazyExpressionEvaluator evaluator;
     private final LazyEvalContext context;
     private final Set<Integer> inflightKeys = new HashSet<>();
-    private final List<Object> originalValues;
+    private final List<?> originalValues;
     private final Map<Integer, Object> evaluatedValues = new HashMap<>();
 
-    public LazyEvalList(LazyExpressionEvaluator evaluator, LazyEvalContext context, List<Object> src) {
+    public LazyEvalList(LazyExpressionEvaluator evaluator, LazyEvalContext context, List<?> src) {
         this.evaluator = evaluator;
         this.context = context;
         this.originalValues = src;